Matthieu Moy wrote: > Jonathan Nieder <jrnieder@xxxxxxxxx> writes: >> If the script is "obviously correct" enough then there is no need >> to manually go through 140 files after that point. > > The script cannot be "obviously correct", as there are a lot of > potential corner-cases (nested `, nesting ` within ", comments, ...). To be a devil's advocate for a moment: * Comments are easy to detect. Remember, we are not trying to handle some adversary sending arbitrary well-formed shell scripts but just the git source code which already follows a consistent style. When I search for /#.*/ in .sh files, every match except for t/test-lib-functions.sh:output=`echo; echo "# Stderr is:"; cat "$stderr"` (which contains a backtick before the # mark) is a comment. * Nested ` is evil. A search for the string \' quickly finds them all, and they are very very rare. The only exception I see is git-svn tests, which independently of everything else is an obvious thing to fix first. * Nesting `` within double-quotes has the same semantics as $() within quotes. I don't think that poses a problem. [...] >> If the only way to get this done is to actually manually review those >> 140 files, I just don't think it's worth it. > > Honnestly, I went through the series once and it wasn't that painful. It is not just the people on the list reviewing now but others in the future wanting to understand whether it is safe to upgrade to a new version or where a bug they have run into came from. The simpler we can make the task of convincing oneself that the patch behaves as described, the better. 140 patches worth of churn once every couple of years is not terrible, but I really don't want to see this becoming a pattern. :/ And I don't see how the upside in this example warrants it. Hoping that clarifies, Jonathan -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html