From: Junio C Hamano Date: Mon, 02 Oct 2017 23:55:01 GMT Subject: Re: [PATCH v2] ref-filter.c: pass empty-string as NULL to atom parsers Message-ID: In-Reply-To: <20171002224052.GR19555@aiede.mtv.corp.google.com> Jonathan Nieder 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 > > 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.