On Thu, Feb 04, 2016 at 04:27:49PM +0100, Johannes Schindelin wrote: > On Thu, 4 Feb 2016, John Keeping wrote: > > > On Thu, Feb 04, 2016 at 02:13:47PM +0100, Johannes Schindelin wrote: > > > Whatever the outcome, the inconsistency must be fixed. > > > > I disagree. Unless there are other changes in the same area, the noise > > isn't worth it. > > > > However, I do think we need to agree on a policy so that new code can be > > consistent. This should then be documented in CodingGuidelines. > > If you want to enforce it in the future, how can you possibly be against > fixing the inconsistency in the existing code? Sorry, I am really unable > to understand this. I avoided quoting CodingGuidelines in my previous message, but it says: - Fixing style violations while working on a real change as a preparatory clean-up step is good, but otherwise avoid useless code churn for the sake of conforming to the style. "Once it _is_ in the tree, it's not really worth the patch noise to go and fix it up." Cf. http://article.gmane.org/gmane.linux.kernel/943020 > Also, we *did* document the policy in the CodingGuidelines: > > As for more concrete guidelines, just imitate the existing code > > So. There you are. By that token, my patch series was the correct thing to > do because the first arithmetic expression introduced into a shell script > in Git's source code had no spaces. This is the first point I made. To take one example, in git-filter-branch.sh there are five occurrences of the sequence "$(("; your patch changes four of them to remove spaces. If we standardise on having spaces only one needs to change: $ git grep -F '$((' origin/master -- git-filter-branch.sh origin/master:git-filter-branch.sh: elapsed=$(($now - $start_timestamp)) origin/master:git-filter-branch.sh: remaining=$(( ($commits - $count) * $elapsed / $count )) origin/master:git-filter-branch.sh: next_sample_at=$(( ($elapsed + 1) * $count / $elapsed )) origin/master:git-filter-branch.sh: next_sample_at=$(($next_sample_at + 1)) origin/master:git-filter-branch.sh: git_filter_branch__commit_count=$(($git_filter_branch__commit_count+1)) I chose git-filter-branch.sh simply because it was touched by this patch set but it is not an outlier in this regard. Some crude statistics across all of git.git: # No space after plus $ git grep -F '$((' | grep '\+[\$0-9]' | wc -l 34 # With space after plus $ git grep -F '$((' | grep '\+ [\$0-9]' | wc -l 96 -- 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