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

[Brendan Byrd]

On Tue, Dec 4, 2012 at 4:29 AM, Peter Rabbitson <rabbit+dbic at rabbit.us> wrote:
> On Tue, Dec 04, 2012 at 12:05:59AM -0500, Brendan Byrd wrote:
>
> Oh I should have executed it to see what exactly it does. Right - then this
> point splits into several problems:
>
> - In this case (number of files) we definitely can not do it in the workdir,
> sorry. There is no easy way to declutter things (make realclean does not touch
> them), and files will linger forever in a repo even if the underlying .pm is
> gone/renamed. Here is a scetch of a functional workaround, will need however
> get to CPAN as a devrel for render-testing:
> https://github.com/dbsrgits/dbix-class/commit/38a597851

As long as the PODs install in the right directories for both user
install and distro creation, I'm fine with that.  I was putting these
in blib, but I ran into other issues.

> - I would strongly recommend not generating POD for the ::Storage family -
> this entire lot need a massive rework, and some things may disappear. Your
> call though.

That's easy enough.  I can add to the skip_classes param.

> - PI needs a way to parse the target POD and exclude methods which are
> undocumented at their origin. And yes, there is a number of "public"
> (non _ed) methods which are in fact undocumented. Maybe they are utility
> (e.g. STORABLE_freeze) or maybe they are deprecated warning-emitting
> methods, which are nevertheless still there (and will be there for a
> while). I am not that concerned over dead links, I am much more
> concerned about re-exposure of hidden stuff.

Hmmm, I might need to cut a release for P:I for that.  We have
skip_underscored for "private" methods and skip_inherits for entire
classes, but nothing to exclude specific methods.  Should be an easy
enough feature to implement, though.

> But before the start of your branch none of these links existed. Hence
> if squashed together the resulting commit will not differ in size. We
> can discuss more of this tonight.

Yeah, I can fix that up in the re-patch.

>> I might have been duplicating the erroneous info from search_related.  I'm
>> beginning to think that we should just remove that last sentence from both
>> of them and let the Inherited Methods sections sort it out.
>
> +1

I'll remove those, then.

> Then let's do just that and fix ::Reading. As a separate commit, either
> before or after the current set of changes (before would be more
> sensible).

Will do.

>> Errr, hmmmm, I might need a code example to work from.  Is @bind_values
>> an array of hashrefs?  I think I can handle the tech writing if I have enough
>> information.
>
> Array of tuple-arrays. Look at any test changed by the commit I
> mentioned above - they all follow the same principle. I was really under
> the impression the commit message was rather exhaustive from an
> explanation POV. In any case - ask as many questions as you need - we
> have not had this documented for 2 years now, it's time.

Well, I guess my confusion is from the commit message describing a hash:

"To fix this all up and provide more flexibility the standard [ $col
=> $val ] was replaced with [ \%args => $val ]. The format of \%args
is currently:"

So, there would be a collection of these array/hash pairs?  Like ( [
\%args => $val ], [ \%args => $val ], ... ) ?

" ( pending in the next patch: [ $val ] === [ {}, $val ] ) "

Was this already implemented?

>> Castaway suggested it, but I agree with her.  Part of this doc change
>> is trying to
>> standardize the language we use when we are referencing variable names.
>> The term "moniker" is just a fancy synonym for "name".  What name?  Row
>> name?  Result name?  Band name?  Dog name?
>>
>> In this case, it's the source name, so why not just call it $source_name,
>> especially since we use the term source_name elsewhere.  Heck, the term
>> "source_name" is even an expected method name in result classes.  Why call
>> them two different things?
>>
>> In fact, I should probably expand that change to remove the term everywhere.
>
> I am on the fence. I would run this particular bit by mst, see what he thinks.

I'll ask him on IRC today.

> Perhaps a more explicit '... (formerly called rows or row objects)...'
> sprinkled around?

I'll think about the language on that one.

> See discussion of 'result' above. Also yes, having the rest say
> ResultSet Object won't be a bad idea imo. But this is even more work, we
> better first agree on a path forward.

Well, playing Devil's Advocate, the term "Result" is ambiguous, but
things like ResultSet or ResultSource are most definitely implied as
objects. I think I'm okay with that direction: renaming just Result.

> We should probably start that if not too difficult. Will reduce the amount
> of "holy shit where do I start", and will clearly delineate the ::Dev::
> namespace.

That's fine, though I think I may split some of these changes into
another topic (doc_fix2?) to get the approved changes out of the way
quicker.  These poor branches have been lingering too long.

>> > In Row when explaining accessors you note it is recommended to use them
>> > because ... and state a weird reason. The reason to use accessors at all
>> > times is for proper encapsulation, so that CMM-style around()s can be
>> > safely written around an accessor. This is mst's theory anyway, ask for
>> > more clarification.
>>
>> I dunno.  I think your reason is a lot weirder than mine, especially to the
>> layman.  (I don't even know what CMM is.)
>
> Not mine actually - I am not shy to use get_column when I believe it is
> warranted. Hence ask mst/abraxxa about it.

Will do.

-- 
Brendan Byrd <Perl at ResonatorSoft.org>
Brendan Byrd <BBYRD at CPAN.org>