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

[Peter Rabbitson]

On Tue, Dec 11, 2012 at 01:37:51PM -0500, Brendan Byrd wrote:
> > Well not quite - key is a *special* attribute, and as such is not documented
> > in Resultset. Perhaps something like:
> > { key => $unique_constraint_name, %attrs(+link) }?
> 
> Why shouldn't we just document this one in ATTRIBUTES?  If it only
> applies to relationship methods, we can just document that in the
> beginning.

Because it is *not* a relationship attribute. It is an *argument* to
find() (on which update_or_create, find_or_create, find_or_new and
the *_related siblings are based). It is not something that search()
ever looks at. Hence the "special" status.

> >> I'm also in favor of expanding resultset_attributes to fix whatever
> >> issues might arise from it.  Like deep nested hash combining, if a
> >> second option is given.
> >
> > I don't follow - what do you mean by "nested hash combining"?
> 
> Just some preliminary ideas I thought of.  rs_attr could have a second
> param that indicates how defaults are merged with existing attributes:
> 
> 0 = whatever we are doing right now (ie: no change from existing behavior)
> 1 = no combining (ie: any %attr will turn off the default)
> 2 = simple combining (ie: if prefetch is in both, the default gets
> clobbered, but the rest will merge)
> 3 = deep nested combining (ie: clobber as little as possible)
> 
> >> Or some sort of attribute that would force no defaults.
> >
> > That would be a solution while looking for a problem :) result_attributes
> > was badly thought through in the first place, and parts of it don't even
> > work properly (and are a massive bitch to fix). See RT#63709 for example.
> > The less the docs and existing machinery encourage their use - the better.
> 
> I don't quite follow why they don't work.  Is this a case of defaults
> being inserted and manipulated too late or too early while other
> processes (like relationship mapping) mess with the %attr hash?  Is
> there a way to implement it "properly" and deprecate the existing
> design?

The simple answer without going into details is: resultset chaining
only works reliably and predictably because of its simple rules:
conditions/joinspecs are always additive, everything else is always
an override (last one wins). Because of this property default_attrs
are not well suite for conditions - one can not shake them off. Nor
are they well suited for other attrs like order_by - one can not shake
it off without supplying a *different* order.

The only way to "fix" this is to do something like what you propose
above, BUT it is a problem in itself because a lot of parts of DBIC
and a lot of extensions *rely* on the fact that once you have an $rs
you can not (easily) get it to "forget" some parts of its conditions.

Basically your proposal isn't a simple addition - it uproots the
very basic principles of resultset behavior. I would like a much more
in-depth proposal which can be subjected to scrutiny and corner-cases
before we start considering implementing it.

In my personal opinion this is a wild goose chase - there is no
tangible benefit to implementing something along these lines. One can
always write *multiple* methods on the resultet in question to achieve
the same effect. It is cleaner, better self-documenting, and much less
intrusive. The effort to implement your idea reliably on the other
hand is *very* substantial. Nevertheless - if you still feel strongly
about the need for such a (mis)feature - draw up a larger proposal. I
just wanted to warn you ahead of time that it will be hard to convince
the core-devs to go forward with something like this.

> 
> > +1. The more the better. There is also a maint/gen_pod_index which has
> > not been touched/used by anyone since 2006 (55a55d31). Perhaps it can
> > be brought back in service?
> 
> Ahhh, wondered what that was, but with so many methods everywhere, it
> might be needed.
> 
> However, it can be implemented in a different fashion using P:I.  P:I
> can be forced to write to a POD (like ResultClass) and it can be
> mapped to basically every module we have.  Then, we can have something
> like Pod::POM edit the header to something else besides "INHERITED
> METHODS".  There would still be a grouping of methods together by
> module, but at least every method would be on one page, easily enough
> for a Ctrl+F.

I was thinking a document along the lines of:

=item update

L<DBIx::Class::CDBICompat::Triggers/update>

L<DBIx::Class::Relationship::CascadeActions/update>

L<DBIx::Class::Relationship::Base/update>

L<DBIx::Class::InflateColumn::File/update>

L<DBIx::Class::Storage::DBI/update>

L<DBIx::Class::Manual::Component/update>

L<DBIx::Class::ResultSet/update>

L<DBIx::Class::Storage/update>

L<DBIx::Class::Row/update>

L<DBIx::Class::Admin/update>

L<DBIx::Class::Ordered/update>

=item delete

...