Confusing/ambiguous Ruby naming scheme

[sebb <se...@gmail.com>]

cttee1 = ASF::Committee.preload
cttee2 = ASF::Committee.load_committee_info


cttee1 gets the LDAP committee info

cttee2 gets info from committee-info.txt


I find this rather confusing, and it seems to me that it is
error-prone and a bit of a maintenance headache, because the Committee
class is now defined in multiple files. If the same method name is
defined in two classes, which one will be used? I suspect it will
depend on the order the files are loaded.

Similar considerations apply to the Person class

Re: Confusing/ambiguous Ruby naming scheme

[Sam Ruby <ru...@intertwingly.net>]

On Wed, Jun 7, 2017 at 5:05 PM, sebb <se...@gmail.com> wrote:
> On 7 June 2017 at 21:05, Shane Curcuru <as...@shanecurcuru.org> wrote:
>> sebb wrote on 6/7/17 3:34 PM:
>>> cttee1 = ASF::Committee.preload
>>> cttee2 = ASF::Committee.load_committee_info
>>>
>>> cttee1 gets the LDAP committee info
>>>
>>> cttee2 gets info from committee-info.txt

My point at the bottom of this email is that cttee1 and ctte2 gets
info from both places.

From a user of this API point of view, a committee is an object.  The
caller of the API doesn't generally need to worry about whether that
data is from LDAP or committtee info or whatever beyond some initial
preloading for performance reasons.

From an implementation point of view, parsing committee-info is a
completely different task than calling LDAP (which has common concerns
such as retrying).

The current library tries to present a unified outward view while
partitioning the implementation based on the data source.

That being said, if you see a better approach: go for it!

- Sam Ruby




>> I think the underlying issue is having sufficiently organized and
>> explanatory developer docs about the ASF module so that new developers
>> can figure out how best to use the models, which offer a lot of data.
>>
>> The whimsy environment is powerful because it has direct access to all
>> sorts of great ASF data.  But for a new developer, it's not easy to
>> figure out if you should try pulling from an ASF::* class by the API,
>> read one of the public_*.json files, or read the source directly
>> yourself.  Similarly, we now have enough developers and independent
>> tools that changes in the ASF:: classes and elsewhere may affect other
>> developers.
>>
>> There certainly are code examples scattered around, but I still just
>> voodoo-copypastaed this line from officers/acreq.cgi when I needed a
>> list of TLP PMC names:
>>
>> pmcs = ASF::Committee.list.map(&:name).sort - NON_PMC_UNIX_GROUPS
>
> I replaced it with ASF::Committee.load_committee_info because the LDAP
> Committee class does not have the correct capitalisation.
> Nor is the list of LDAP committee groups the canonical list of PMCs.
>
>> For my case, I don't care about details or even data freshness: I simply
>> want the fastest accurate list of all TLP names, (and separately, the
>> accurate list of current podling names).
>
> In which case committee-info.json would be sufficient.
> [One can use the approach in the collate-minutes script which loads
> the local copy if present, and fetches the URL if not.]
>
>> - Do we have a preferred ruby class doc style?
>>
>> - Is there anything else really important to document besides the ASF::
>> gem/classes and the content/structure of the public*.json files (which
>> are used by other websites now too)?
>
> That's really a separate issue.
>
>>>
>>> I find this rather confusing, and it seems to me that it is
>>> error-prone and a bit of a maintenance headache, because the Committee
>>> class is now defined in multiple files. If the same method name is
>>> defined in two classes, which one will be used? I suspect it will
>>> depend on the order the files are loaded.
>>>
>>> Similar considerations apply to the Person class
>>>
>>
>>
>> --
>>
>> - Shane
>>   https://www.apache.org/foundation/marks/resources

Re: Confusing/ambiguous Ruby naming scheme

[sebb <se...@gmail.com>]

On 7 June 2017 at 21:05, Shane Curcuru <as...@shanecurcuru.org> wrote:
> sebb wrote on 6/7/17 3:34 PM:
>> cttee1 = ASF::Committee.preload
>> cttee2 = ASF::Committee.load_committee_info
>>
>>
>> cttee1 gets the LDAP committee info
>>
>> cttee2 gets info from committee-info.txt
>
> I think the underlying issue is having sufficiently organized and
> explanatory developer docs about the ASF module so that new developers
> can figure out how best to use the models, which offer a lot of data.
>
> The whimsy environment is powerful because it has direct access to all
> sorts of great ASF data.  But for a new developer, it's not easy to
> figure out if you should try pulling from an ASF::* class by the API,
> read one of the public_*.json files, or read the source directly
> yourself.  Similarly, we now have enough developers and independent
> tools that changes in the ASF:: classes and elsewhere may affect other
> developers.
>
> There certainly are code examples scattered around, but I still just
> voodoo-copypastaed this line from officers/acreq.cgi when I needed a
> list of TLP PMC names:
>
> pmcs = ASF::Committee.list.map(&:name).sort - NON_PMC_UNIX_GROUPS

I replaced it with ASF::Committee.load_committee_info because the LDAP
Committee class does not have the correct capitalisation.
Nor is the list of LDAP committee groups the canonical list of PMCs.

> For my case, I don't care about details or even data freshness: I simply
> want the fastest accurate list of all TLP names, (and separately, the
> accurate list of current podling names).

In which case committee-info.json would be sufficient.
[One can use the approach in the collate-minutes script which loads
the local copy if present, and fetches the URL if not.]

> - Do we have a preferred ruby class doc style?
>
> - Is there anything else really important to document besides the ASF::
> gem/classes and the content/structure of the public*.json files (which
> are used by other websites now too)?

That's really a separate issue.

>>
>> I find this rather confusing, and it seems to me that it is
>> error-prone and a bit of a maintenance headache, because the Committee
>> class is now defined in multiple files. If the same method name is
>> defined in two classes, which one will be used? I suspect it will
>> depend on the order the files are loaded.
>>
>> Similar considerations apply to the Person class
>>
>
>
> --
>
> - Shane
>   https://www.apache.org/foundation/marks/resources

Re: Confusing/ambiguous Ruby naming scheme

[Shane Curcuru <as...@shanecurcuru.org>]

sebb wrote on 6/7/17 3:34 PM:
> cttee1 = ASF::Committee.preload
> cttee2 = ASF::Committee.load_committee_info
> 
> 
> cttee1 gets the LDAP committee info
> 
> cttee2 gets info from committee-info.txt

I think the underlying issue is having sufficiently organized and
explanatory developer docs about the ASF module so that new developers
can figure out how best to use the models, which offer a lot of data.

The whimsy environment is powerful because it has direct access to all
sorts of great ASF data.  But for a new developer, it's not easy to
figure out if you should try pulling from an ASF::* class by the API,
read one of the public_*.json files, or read the source directly
yourself.  Similarly, we now have enough developers and independent
tools that changes in the ASF:: classes and elsewhere may affect other
developers.

There certainly are code examples scattered around, but I still just
voodoo-copypastaed this line from officers/acreq.cgi when I needed a
list of TLP PMC names:

pmcs = ASF::Committee.list.map(&:name).sort - NON_PMC_UNIX_GROUPS

For my case, I don't care about details or even data freshness: I simply
want the fastest accurate list of all TLP names, (and separately, the
accurate list of current podling names).

- Do we have a preferred ruby class doc style?

- Is there anything else really important to document besides the ASF::
gem/classes and the content/structure of the public*.json files (which
are used by other websites now too)?

> 
> I find this rather confusing, and it seems to me that it is
> error-prone and a bit of a maintenance headache, because the Committee
> class is now defined in multiple files. If the same method name is
> defined in two classes, which one will be used? I suspect it will
> depend on the order the files are loaded.
> 
> Similar considerations apply to the Person class
> 


-- 

- Shane
  https://www.apache.org/foundation/marks/resources

Re: Confusing/ambiguous Ruby naming scheme

[sebb <se...@gmail.com>]

On 7 June 2017 at 21:34, Sam Ruby <ru...@intertwingly.net> wrote:
> On Wed, Jun 7, 2017 at 3:34 PM, sebb <se...@gmail.com> wrote:
>> cttee1 = ASF::Committee.preload
>> cttee2 = ASF::Committee.load_committee_info
>>
>> cttee1 gets the LDAP committee info
>>
>> cttee2 gets info from committee-info.txt
>>
>> I find this rather confusing, and it seems to me that it is
>> error-prone and a bit of a maintenance headache, because the Committee
>> class is now defined in multiple files. If the same method name is
>> defined in two classes, which one will be used? I suspect it will
>> depend on the order the files are loaded.
>>
>> Similar considerations apply to the Person class
>
> I'm not sure I understand this comment.  In particular, do you have an
> example where the same method name is defined for the same class in
> two different files?

AFAIK there are no clashes - yet.

However if there is a need to add a new method to one of the classes,
one has to check if the name has already been used in one of the other
class definitions.

And it makes code re-organisation harder.
For example, 'load_committee_info' performs much the same function as
the 'preload' methods in other classes.
For consistency one might consider renaming it as 'preload' ...

It's also very confusing having the same class name for LDAP commitee
group info and PMC info derived from CI.txt etc.

This is less of an issue for the Person class does appear to describe
a single type of entity.

> Meanwhile, classes list ASF::Commitee and ASF::Person are constructed
> from various sources.

Yes, and again there is the potential issue of different sources
accidentally defining the same method or field with a different
meaning.
The Person class is currently defined in 8 source files.
A maintenance headache.

> But one thing you can count on: if you find a
> given person or committee or whatever, and still have a handle for
> that object, and try to find it again, you will get the same object.

That is interesting but tangential to this thread.

Re: Confusing/ambiguous Ruby naming scheme

[Sam Ruby <ru...@intertwingly.net>]

On Wed, Jun 7, 2017 at 3:34 PM, sebb <se...@gmail.com> wrote:
> cttee1 = ASF::Committee.preload
> cttee2 = ASF::Committee.load_committee_info
>
> cttee1 gets the LDAP committee info
>
> cttee2 gets info from committee-info.txt
>
> I find this rather confusing, and it seems to me that it is
> error-prone and a bit of a maintenance headache, because the Committee
> class is now defined in multiple files. If the same method name is
> defined in two classes, which one will be used? I suspect it will
> depend on the order the files are loaded.
>
> Similar considerations apply to the Person class

I'm not sure I understand this comment.  In particular, do you have an
example where the same method name is defined for the same class in
two different files?

Meanwhile, classes list ASF::Commitee and ASF::Person are constructed
from various sources.  But one thing you can count on: if you find a
given person or committee or whatever, and still have a handle for
that object, and try to find it again, you will get the same object.

Try, for example:

sebb = ASF::Person.find('sebb')
p sebb
ASF::Person.preload('cn')
p sebb

Keeping a handle on the object can therefore be very important for
performance reasons.  If you let it go, and the storage is garbage
collected for any reason, the next time you access that
person/committee/whatever it will need to go back out to LDAP.  One
fetch of every 'cn' is much more efficient that an individual fetch
for every person; and is much more efficient than multiple fetches for
every person.

That's why you will see code like the following:

people = ASF::Person.preload('cn', group.members)

... even if 'people' is never explicitly referenced by the code again,
keeping a handle on those objects will prevent them from being garbage
collected until the people variable itself goes out of scope.

- Sam Ruby