[DBIx-Class-Devel] Pull Request: topic/docs-pod-inherit

[Peter Rabbitson]

On Mon, Dec 10, 2012 at 05:19:02PM -0500, Brendan Byrd wrote:
> Okay, this branch is ready for re-review.
> 
> http://git.shadowcat.co.uk/gitweb/gitweb.cgi?p=dbsrgits/DBIx-Class.git;a=shortlog;h=refs/heads/topic/docs-pod-inherit

Excellent, excellent work. Some pieces still need an extra touch, so I 
applied some as-is ans some commits got split. All of this is detailed 
below. The "leftovers" were pushed to a new branch 
topic/docfixes_pending.

Also in 2b3ac681 (which is already applied) you based the pod of
DBIx::Class::Manual::ResultClass on earlier work of mine found in
14f8d7dbc. However you left out the initialization docs - please
add them back in a subsequent commit. Also if PI allows it, it may
be nice to separate the ResultSource::Proxy inheritance from
everything else, the way I planned to do it in 14f8d7dbc. In any
case this details further work - the actual stuff is already in
master.

States of branches now is:

master - contains bulk of your changes already

topic/docfixes_pending - contains what didn't make it

topic/docs-pod-inherit - your original branch for review, delete when no
  longer needed

pod/result_class - contains 14f8d7dbc and the missing POD as described
 in last paragraph above

topic/rs_doc_fix2 - an older branch of yours - seems to contain nothing
  that has not been already done - review and delete


Review/problemlist follows:


=== 2b3ac68
The first commit 2b3ac68 was applied with very minor changes:
`git diff --color 2b3ac68 3d4c5a8`

=== 0cab3cc
0cab3cc30 still has a number of problems so I applied it *partially*.
Everything was taken as-ise, except for Relationship::Base and Resultset,
which were not applied at all. I re-pushed your branch with the leftovers.
The problems that need fixing are as follows:

* In Relationship::Base you normally link \%attrs, however you failed to do
so in update_or_create_related (you only mention key => ). Also above it in
find_or_create_related you *do not* mention key =>. Please adjust.

* The signature of Relationship::Base add_to_$rel is plain wrong (while
set_$rel is correct)

* in Resultset::new() you do not link to \%attrs as everywhere else

* in Resultset $cond has inconsistent linking to SQLMaker (sometimes it
does sometimes it doesn't). I am also not sure if linking to SQLMaker is
of any value... especially given the DQ underpinnings change. Perhaps not
link for the time being at all, or add a "search arguments" document 
somewhere in ::Manual::, which in turn links to the appropriate resources
of the day?

* Resultset::update_or_create has the same problem with key/attrs as above

=== dc26bfc45
Applied partially - the [ $val ] === [ {}, $val ] part is not as simple
as you may think. Consider:

perl -Ilib -It/lib -MDBICTest -e 'DBICTest->init_schema->resultset("Artist")->search({ name => \["=", [ "foo" ] ]})->as_query'
DBIx::Class::ResultSet::as_query(): [SQL::Abstract::_assert_bindval_matches_bindtype] Fatal: bindtype 'columns' selected, you need to pass: [column_name => bind_value] at -e line 1

Basically binds are interrogated within SQLA way before they make it to
bind-time in ::Storage::DBI. So I took this part away and left the rest
of the docs in.

Also you had incorrectly linked the BIND documentation to search_literal -
I fixed this in d4a9feb6.

=== d791c8cd
This was applied as-is

=== 7a82dc29
This was not applied - any documentation of default_attributes must be
combined with a massive warning of the caveats of doing so. Every time
someone asks "how do I set default attrs", they come back a day later
and ask "but how do I create a resultset just *this one time* that does
not have my default attrs (the answer is - you can't). Hence an
alternative approach of project-local methods that return new resultsets
with the desired attributes is always preferred.

> I'd like to cut a dev release to make sure the .generated_pod stuff
> works.  And yes, I caught your directory change and OptDeps work.  (I
> was actually working on that File::Spec change, but you beat me to
> it.)

As stated earlier - when you need one jfdi (check on metacpan what is
the latest current devrel first, as one may have been uploaded and
then deleted)

Cheers