John Keeping <john@xxxxxxxxxxxxx> writes: > 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 > > 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: I agree that our codebase seems to have a moderately strong preference for having SP around binary operators, i.e. $term1 * $term2 + $term3 over not having one: $term1*$term2+$term3 and I think that is OK to declare it as our style, primarily because that is how we encourage to write our expressions in C, as you mentioned earlier. One thing I was wondering about the $(( ... )) syntax while reading this thread was about the SP around the expression, i.e. var=$(( $term1 * $term2 + $term3 )) vs var=$(($term1 * $term2 + $term3)) I personally do not have strong preference between the two, but I have a vague impression that we preferred the former because somebody in the past gave us a good explanation why we should. "git grep" however seems to tell us that we are not clearly decided between the two, so I probably am misremembering it and there is no preference either way. In any case, it is good to catch a patch that mixes style changes and other things during a review, and it also is good to clean up the style before you start working on a specific part of the code to make gradual improvement without stepping on others' toes. Thanks. -- 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