refactor - combine physicalprocessorcount.rb and processor.rb in facter?

10 views
Skip to first unread message

Alex Harvey

unread,
Dec 20, 2012, 8:54:16 AM12/20/12
to puppe...@googlegroups.com
Hi all,

I've sat down to write physicalprocessorcount for HP-UX and found I'd like to call a method machinfo that I've put in util/processor.rb.  The processorX and processorcount facts however live in lib/facter/processor.rb whereas physicalprocessorcount lives in lib/facter/physicalprocessorcount.rb and are self-contained.

I could, I suppose, have physicalprocessorcount.rb also require util/processor.rb but it makes more sense to me to have all processor-related facts inside processor.rb and let that continue to require methods in util/processor.rb.  Thus it makes sense to me to refactor first and combine physicalprocessorcount.rb and processor.rb in a single file.  The latter file, after all, already contains two processor-related facts - why should physicalprocessorcount be special?

However, before I venture too far down this path I guess I should check if anyone objects - so would it be okay if my patch for #17520 includes one commit that refactors as proposed and another that adds the HP-UX physicalprocessorcount fact?

TIA,
Alex

Andy Parker

unread,
Dec 20, 2012, 1:30:39 PM12/20/12
to puppe...@googlegroups.com
On Thu, Dec 20, 2012 at 5:54 AM, Alex Harvey <alexh...@gmail.com> wrote:
Hi all,

I've sat down to write physicalprocessorcount for HP-UX and found I'd like to call a method machinfo that I've put in util/processor.rb.  The processorX and processorcount facts however live in lib/facter/processor.rb whereas physicalprocessorcount lives in lib/facter/physicalprocessorcount.rb and are self-contained.

I could, I suppose, have physicalprocessorcount.rb also require util/processor.rb but it makes more sense to me to have all processor-related facts inside processor.rb and let that continue to require methods in util/processor.rb.  Thus it makes sense to me to refactor first and combine physicalprocessorcount.rb and processor.rb in a single file.  The latter file, after all, already contains two processor-related facts - why should physicalprocessorcount be special?


Normally the facts are kept in a file that matches the name of the fact. This is a convention that is not always followed, but that is usually in cases where we need to dynamically generate facts (such as to list the network interfaces). One of the reasons for tying the filename to the fact name is that when facter is asked for a single fact it will try to load just that specific file if it exists and then fall back to loading everything to find the fact.

This situation can cause surprising results at times: http://projects.puppetlabs.com/issues/15353. Because of that bug, I think keeping the facts in a file that matches the name of the facts added by that file is what we need to do.
 
However, before I venture too far down this path I guess I should check if anyone objects - so would it be okay if my patch for #17520 includes one commit that refactors as proposed and another that adds the HP-UX physicalprocessorcount fact?

TIA,
Alex

--
You received this message because you are subscribed to the Google Groups "Puppet Developers" group.
To view this discussion on the web visit https://groups.google.com/d/msg/puppet-dev/-/3Zl9n8F2GjkJ.
To post to this group, send email to puppe...@googlegroups.com.
To unsubscribe from this group, send email to puppet-dev+...@googlegroups.com.
For more options, visit this group at http://groups.google.com/group/puppet-dev?hl=en.

Reply all
Reply to author
Forward
0 new messages