On Nov 15, 2010, at 10:25 PM, Jesse Wolfe wrote:
> While I certainly appreciate your willingness to get your hands dirty with
> this code, I hesitate to accept a patch that doesn't produce any change in
> functionality.
If I have the time I will change the functionality. I am ok with it to let
it in my fork and resend this patch with follow ups if I took the time.
> Some of your edits are things we generally agree on: removing unused
> variables, removing the unnecessary subclassing of Exception;
> but many of the others are debatable: not everyone agrees that "unless" is
> more readable than "if not", I'm not sure that DSLOCAL_DIR is the best way to
> extract that directory name, and we pretty consistently use "self.method"
> rather than "class << self" in the puppet codebase.
I am ok with your opinions, but I think it should be a documented decision.
I count files that use of def self. and class << self.
# '.' is the root of puppet src
$ grep 'def self\.' -rn * |awk '{print $1}' | awk -F ':' '{print $1}' | sort |
uniq | wc -l
186
$ grep 'class << self' -rn * |awk '{print $1}' | awk -F ':' '{print $1}' | sort
| uniq | wc -l
48
The constant was chosen, because I did not have an idea to add a member that
can be used by a manifest. I just have to read another provider and it will
not take much time. I think I sent this patch too early.
> There's no official puppet policy on this, as far as I know, but I'm inclined
> to say that we should only accept cleanup/refactoring patches if they are
> part of a series of changes that also adds or fixes functionality.
> Does anyone else have an opinion on the matter?
I am ok with it, but I think it should be a documented decision. I can open
another ticket if I have done a new feature, so feel free to close it.
All the best, Sandor Szücs
--
--
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.
