2009/6/8 Felix Schäfer <[email protected]>:
>
> And an assignment is exactly what I was aiming for: the assignment
> "fails" if the right hand doesn't exist, which is the condition in
> which I don't want to go into the branch, if the right hand exists, I
> get the assignment "for free" (the only pitfall here would be if a
> possible value of the assignment was 'false', which shouldn't happen).
I personally find that style to be more confusing as the intent isn't
as clear - I got misled instantly!
I'm not sure we need to be worrying about the optimisation here, and
I'd tend to fall on the side in favour of clarity.
Reading over the existing fact, it looks like we're trying to be too
generic, we shouldn't even try /proc/uptime on Solaris and I assume
AIX too.
I think we might be better here with splitting this out into two
confined resolvers, one for linux systems with /proc/uptime (which has
been around for a long time) and one for non-linux systems. I think
this will achieve the only do things once desire of your bug report
and clean up the code at the same time.
eg:
Facter.add(:uptime) do
confine :kernel => %w{SunOS AIX}
setcode do
#generic uptime command processing
end
end
Facter.add(:uptime) do
confine :kernel => :linux
setcode do
#/proc/uptime parsing and manipulation
end
end
end
> If you prefer to separate this into two distinct steps, I can rework
> it accordingly.
>> White space noise pollution.
>
> Yeah, well, I'm often a little maniac regarding needless white space
> at the end of lines and so on, I'll try to keep it in bounds next time
> around, sorry.
It's ok just do it as a seperate commit rather than with the logic,
this is purely so that code reviewers can focus on discreet changes.
> Regarding the complex logic: I wasn't sure if it should go into facter/
> uptime or facter/util/uptime, my gripe with the later being getting
> bounced 3 or 4 times around the 2 files.
> Anyway, this brings me to a question I still wanted to discuss,
..
> It seems I have diverged a little bit here. If the answer is longer
> than "already discussed" or "won't do because", please move to a new
> thread.
The discussion is fine - we need to discuss the design of things more.
I'll follow up on that point in a separate mail so it doesn't get lost
in the patch discussion.
> A last word concerning the patch: I'm trying to get up to speed on
> ruby as well as on facter, and I'd like to help facter/puppet in the
> process, I'd appreciate a little more constructive comments.
Sure, apologies if I was a little abrupt I just did a pass through
review. Unfortunately some of the facts are a little messy
and I'm trying to get to code with a better level of abstraction as we
move to 1.6 and 2.0. We're always grateful to receive patches, sorry
that didn't come across.
Maybe some better contributor focused docs for facter would be useful,
it's on my TODO and I'll try to get to this week.
Paul
--~--~---------~--~----~------------~-------~--~----~
You received this message because you are subscribed to the Google Groups
"Puppet Developers" group.
To post to this group, send email to [email protected]
To unsubscribe from this group, send email to
[email protected]
For more options, visit this group at
http://groups.google.com/group/puppet-dev?hl=en
-~----------~----~----~----~------~----~------~--~---
