On Tue, Oct 03, 2017 at 08:55:01AM +0900, Junio C Hamano wrote: > Jonathan Nieder <jrnieder@xxxxxxxxx> writes: > > > The above does a nice job of explaining > > > > - what this change is going to do > > - how it's good for the internal code structure / maintainability > > > > What it doesn't tell me about is why the user-facing effect won't > > cause problems. Is there no atom where %(atom:) was previously > > accepted and did something meaningful that this may break? > > That is, was there any situation where %(atom) and %(atom:) did two > differnt things and their differences made sense? > > > Looking at the manpage and code, I don't see any, so for what it's > > worth, this is > > > > Reviewed-by: Jonathan Nieder <jrnieder@xxxxxxxxx> > > > > but for next time, please remember to discuss regression risk in > > the commit message, too. > > Yes, I agree that it is necessary to make sure somebody looked at > the issue _and_ record the fact that it happened. Thanks for doing > that already ;-) > > I also took a look at the code and currently we seem to abort, > either with "unrecognised arg" (e.g. "refname:") or "does not take > args" (e.g. "body"), so we should be OK, I'd think. Thank you both for the helpful pointers. I will make sure to include a more thorough review of potential breakage in existing code given a particular change. With respect to this particular patch, I agree with Jonathan and Junio that there are no places in ref-filter.c that would be affected by this change, and that it should be able to be applied cleanly without breakage. -- - Taylor
