Hi Matt! On Nov 16, 2010, at 9:11 PM, Matt Robinson wrote: > > 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.
Sorry for my too long patch. I was not aware of the function of the rake task in combination with git. It really makes sense to have really small refactoring steps, that are obvious like renaming a method or extracting a method. When I had committed each step, then you and Paul had not so much work and I had not mixed it with Exception handling. > I've updated the development lifecycle document to specify that > refactor commits should be done in separate commits to improve diff > readability. Yeah, thanks! It will help other people in the future. > 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 Thanks for your work. > 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. Just merge your branch I read it and say. +1 The constant drop is ok, because it is not needed and just leaks around. > Then you could make the exception change as a separate commit if you > still think it's useful. I will do it sooner or later. @all It would be nice if you (community + puppetlabs) can think about a global exception policy and write it down. For me it is obvious to use Puppet::Error for normal errors, and only have local error or exception classes if the code use it for something special. Maybe it is the way the project should go, maybe not. The decision should be documented so all can have the same idea of this tool. A project newbie will get faster into the project and you can answer emails with a hint to links if anyone do not follow the guide. 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.
