On Nov 16, 2010, at 12:11 PM, Matt Robinson wrote: > On Mon, Nov 15, 2010 at 11:41 PM, Luke Kanies <[email protected]> wrote: >> I think the exact opposite should be true - accept cleanup and refactoring >> patches wherever and whenever we can. Heck, I think we should give extra >> credit for refactoring patches that change no behaviour at all. >> As to whether all of his changes meet our standards, his points about our >> standards not being published is a good point, and I think it makes sense to >> use his patch as a forcing function to publish what bits are not yet >> decided. > > Paul and I spent a little while looking at this refactor, and in the > end we agree the changes are good, but it was REALLY hard to tell that > from the diff. This was because code was moved around, changed, and > some non-refactoring work (changing exception raised) was all done in > the same commit.
Thanks, that makes a lot of sense, and I appreciate the clarification. I agree that refactor commits should be as tight as possible and in a single commit. > I've updated the development lifecycle document to specify that > refactor commits should be done in separate commits to improve diff > readability. Even better. > Sandor, I created an example of how we think the changes you made > could have been done as a couple commits. > > https://github.com/mmrobins/puppet/tree/ticket/next/5079 > > I didn't make the exception rename change since that wasn't a > refactor, or move "Default" out to a constant since that was a > refactor that currently isn't useful. If you'd like to change your > patch to make the commits clearer and resubmit we'd appreciate that, > or if you like we can just merge in my branch as we've reworked it. > Then you could make the exception change as a separate commit if you > still think it's useful. > Matt > > -- > 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. > -- A citizen of America will cross the ocean to fight for democracy, but won't cross the street to vote in a national election. --Bill Vaughan --------------------------------------------------------------------- Luke Kanies -|- http://puppetlabs.com -|- +1(615)594-8199 -- 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.
