[DBIx-Class-Devel] topic/rework_intro

[fREW Schmidt]

1) as usual, you included an unrelated change.  The fact that your
commit message was multiline should have told you this.  Split the
change to the relationship out.

2) at least make it A Result class ~~ Table, since it's not really the
same

3) "Let's say we defined a L<Result classes" <-- woops :)

4) '=head2 Search results are returned as "Rows"' There are a lot of
things I'm worried about in this hunk.  While it's true that users
define their result classes and their result sources in the same
place, they really aren't the same thing at all.  If you tried to call
$row->has_many(...) it wouldn't work.  I would let the user forget
about this entirely personally, but that's up to you.  I'd also add
that you should add methods to your result class for reusable bits of
behaviour on your rows.

5) While we're making changes, "native DBIx::Class tree", what even is
this thing?  Maybe it should be DBIx::Class Schema, but tree can mean
so many things, and what is native about it?

6) I'd recommend not even telling people about
DBIx::Class::Schema::Versioned.  It was fine for the stone ages, but
people might think that because it is so simple it is the right tool
for them, but then it doesn't support very many use cases etc.  Just
leave it out for noobs.  The main reasons are enumerated here:
http://search.cpan.org/dist/DBIx-Class/lib/DBIx/Class/Schema/Versioned.pm#SEE_ALSO

7) as mentioned in the docs to load_classes
(http://search.cpan.org/~abraxxa/DBIx-Class-0.08209/lib/DBIx/Class/Schema.pm#load_classes)
load_namespaces is the preferred tool, and thus I think all that needs
to be mentioned in a getting started doc.

8) I hesitantly posit that replacing the standard boilerplate with
DBIx::Class::Candy could be a boon for new users, but I'd rather
someone other than me say that.  (mst uses it.)

9) I vote leave out defaults, like is_nullable => 0.  They are just
noise.

10) Go ahead and remove the "yet" from the hunk after the columns.
It's incredibly unlikely that DBIC will start using already existing
fields in the future.

11) maybe link to
http://search.cpan.org/~abraxxa/DBIx-Class-0.08209/lib/DBIx/Class/Manual/Intro.pod#The_Significance_and_Importance_of_Primary_Keys
for the PK.

12) for Relationships, it just sounds really wrong in general.
"L<belongs_to|DBIx::Class::Relationship/belongs_to> to describe a
column which contains an ID of another Table", it could be multiple
columns.  "has_many> to make a predefined accessor for fetching objects
that contain this Table's foreign key" wait why doesn't the other one
make an accessor?  (protip: it does.)

Aside from those things this is definitely a step forward!  Good job!
:)

On Thu, Mar 14, 2013 at 10:59:58AM -0400, Brendan Byrd wrote:
> Topic branch ready for review.
>
> --
> Brendan Byrd <Perl at ResonatorSoft.org>
> Brendan Byrd <BBYRD at CPAN.org>

> _______________________________________________
> DBIx-Class-Devel mailing list
> DBIx-Class-Devel at lists.scsys.co.uk
> http://lists.scsys.co.uk/cgi-bin/mailman/listinfo/dbix-class-devel


--
fREW Schmidt
http://blog.afoolishmanifesto.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: not available
Url : http://lists.scsys.co.uk/pipermail/dbix-class-devel/attachments/20130314/ab8125e6/attachment.pgp