On 08/18/2014 06:06 PM, Daniel P. Berrange wrote: > On Mon, Aug 18, 2014 at 11:27:39AM -0400, Doug Hellmann wrote: >> >> On Aug 18, 2014, at 10:15 AM, Daniel P. Berrange <berra...@redhat.com> wrote: >> >>> On Mon, Aug 18, 2014 at 07:57:28AM +1000, Michael Still wrote: >>>> My recollection is that this was a request from the oslo team, but it >>>> was so long ago that I don't recall the details. >>>> >>>> I think the change is low value, so should only be done when someone >>>> is changing the logging in a file already (the log hinting for >>>> example). >>> >>> The "lazy conversion" approach really encourages bad practice and is very >>> wasteful for developers/reviewers. In GIT commit guidelines we explicitly >>> say not to make code cleanups in their code that are unrelated to the >>> feature/bug being addressed. When we have done lazy conversion for this >>> kind of thing, I've seen it waste a hell of alot of time for developers. >>> People are never entirely clear which is the preferred style, so they >>> end up just making a guess which will often be wrong. So now we consume >>> scarce reviewer time pointing this out to people over & over & over again, >>> and waste developer time having them re-post their patches again. >>> >>> If we want to change LOG.warning to LOG.warn, we should do a single >>> patch with a global search & replace to get the pain over & done with >>> as soon as possible, then enforce it with a hacking rule. No reviewer >>> time gets wasted and developers will see their mistake right away. >> >> The only issue I can see with that approach is causing existing patches >> to have to be rebased. That can be mitigated by making these sorts of >> cleanups at a time when other feature changes aren’t going to be under >> development, though. > > The pessimist in me says that the majority of existing patches are going > have to be rebased many more times for unrelated reasons already, so this > won't be as bad as you might fear. It would make sense to avoid doing > these kind of cleanups immediately before any release milestones though, > since there is no sense in intentionally inflicting this pain at the > worst possible time :-) > > So IMHO there is a tiny window for someone to propose this for Juno > right now if reviewers commit to merging it quickly. Otherwise any > conversion should wait until Kilo opens, and don't attempt to do a > piece-by-piece conversion in the meantime.
I agree with Daniel here. I'd rather have a single, ninja-approved patch changing all matches than several small patches that introduce unrelated changes to replace warn with warning. An additional benefit is that the single-replacement patch may also be used as a reference for future commits using `warn` instead of `warning`. My $0.02, Flavio -- @flaper87 Flavio Percoco _______________________________________________ OpenStack-dev mailing list OpenStack-dev@lists.openstack.org http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
