{"thread":{"id":"36558","subject":"[PATCH 0/8] Update the CodingGuidelines","startedAt":"2014-04-30T21:45:04Z","lastAt":"2014-05-02T21:00:42Z","messageCount":30,"participants":["Junio C Hamano","Stefan Beller","David Kastrup","Jeff King","Felipe Contreras","brian m. carlson"],"isPatch":true,"patchVersion":1,"patchTotal":8},"messages":[{"id":"240375","messageId":"1398894312-30763-1-git-send-email-gitster@pobox.com","threadId":"36558","inReplyTo":null,"subject":"[PATCH 0/8] Update the CodingGuidelines","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2014-04-30T21:45:04Z","receivedAt":"2014-04-30T21:45:04Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"After seeing a large torrent of patches from Elia, I was re-reading\nthe CodingGuidelines to see if our preference of $( ... ) over `...`\nwas properly described, and then I spotted a typo and also lack of\nexamples on a few points that I recall pointing out during recent\nreviews, which grew to this series.\n\nJunio C Hamano (8):\n CodingGuidelines: typofix\n CodingGuidelines: give an example for case/esac statement\n CodingGuidelines: give an example for redirection\n CodingGuidelines: give an example for control statements\n CodingGuidelines: give an example for shell function preamble\n CodingGuidelines: call the conditional statement \"if ()\", not \"if()\"\n CodingGuidelines: on comparison\n CodingGuidelines: once it is in, it is not worth the code churn\n\n Documentation/CodingGuidelines | 81 ++++++++++++++++++++++++++++++++++++++----\n 1 file changed, 75 insertions(+), 6 deletions(-)\n\n-- \n2.0.0-rc1-355-gd6d6511\n"},{"id":"240376","messageId":"1398894312-30763-2-git-send-email-gitster@pobox.com","threadId":"36558","inReplyTo":"1398894312-30763-1-git-send-email-gitster@pobox.com","subject":"[PATCH 1/8] CodingGuidelines: typofix","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2014-04-30T21:45:05Z","receivedAt":"2014-04-30T21:45:05Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"The sentence lacked the necessary verb.\n\nSigned-off-by: Junio C Hamano \n---\n Documentation/CodingGuidelines | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/Documentation/CodingGuidelines b/Documentation/CodingGuidelines\nindex f424dbd..fdf6269 100644\n--- a/Documentation/CodingGuidelines\n+++ b/Documentation/CodingGuidelines\n@@ -15,7 +15,7 @@ code. For Git in general, three rough rules are:\n let's use it\".\n \n Again, we live in the real world, and it is sometimes a\n- judgement call, the decision based more on real world\n+ judgement call, the decision is based more on real world\n constraints people face than what the paper standard says.\n \n Make your code readable and sensible, and don't try to be clever.\n-- \n2.0.0-rc1-355-gd6d6511\n"},{"id":"240377","messageId":"1398894312-30763-3-git-send-email-gitster@pobox.com","threadId":"36558","inReplyTo":"1398894312-30763-1-git-send-email-gitster@pobox.com","subject":"[PATCH 2/8] CodingGuidelines: give an example for case/esac statement","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2014-04-30T21:45:06Z","receivedAt":"2014-04-30T21:45:06Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Signed-off-by: Junio C Hamano \n---\n Documentation/CodingGuidelines | 12 +++++++++++-\n 1 file changed, 11 insertions(+), 1 deletion(-)\n\ndiff --git a/Documentation/CodingGuidelines b/Documentation/CodingGuidelines\nindex fdf6269..9b103cb 100644\n--- a/Documentation/CodingGuidelines\n+++ b/Documentation/CodingGuidelines\n@@ -34,7 +34,17 @@ For shell scripts specifically (not exhaustive):\n \n - We use tabs for indentation.\n \n- - Case arms are indented at the same depth as case and esac lines.\n+ - Case arms are indented at the same depth as case and esac lines,\n+ like this:\n+\n+\tcase \"$variable\" in\n+\tpattern1)\n+\t\tdo this\n+\t\t;;\n+\tpattern2)\n+\t\tdo that\n+\t\t;;\n+\tesac\n \n - Redirection operators should be written with space before, but no\n space after them. In other words, write 'echo test >\"$file\"'\n-- \n2.0.0-rc1-355-gd6d6511\n"},{"id":"240379","messageId":"1398894312-30763-4-git-send-email-gitster@pobox.com","threadId":"36558","inReplyTo":"1398894312-30763-1-git-send-email-gitster@pobox.com","subject":"[PATCH 3/8] CodingGuidelines: give an example for redirection","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2014-04-30T21:45:07Z","receivedAt":"2014-04-30T21:45:07Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Signed-off-by: Junio C Hamano \n---\n Documentation/CodingGuidelines | 6 ++++++\n 1 file changed, 6 insertions(+)\n\ndiff --git a/Documentation/CodingGuidelines b/Documentation/CodingGuidelines\nindex 9b103cb..1e0c4cf 100644\n--- a/Documentation/CodingGuidelines\n+++ b/Documentation/CodingGuidelines\n@@ -53,6 +53,12 @@ For shell scripts specifically (not exhaustive):\n redirection target in a variable (as shown above), our code does so\n because some versions of bash issue a warning without the quotes.\n \n+\tcat hello > world < universe\t# incorrect\n+\tcat hello >world $world\t\t# incorrect\n+\techo hello >\"$world\"\t\t# correct\n+\n - We prefer $( ... ) for command substitution; unlike ``, it\n properly nests. It should have been the way Bourne spelled\n it from day one, but unfortunately isn't.\n-- \n2.0.0-rc1-355-gd6d6511\n"},{"id":"240378","messageId":"1398894312-30763-5-git-send-email-gitster@pobox.com","threadId":"36558","inReplyTo":"1398894312-30763-1-git-send-email-gitster@pobox.com","subject":"[PATCH 4/8] CodingGuidelines: give an example for control statements","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2014-04-30T21:45:08Z","receivedAt":"2014-04-30T21:45:08Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Signed-off-by: Junio C Hamano \n---\n Documentation/CodingGuidelines | 11 +++++++++++\n 1 file changed, 11 insertions(+)\n\ndiff --git a/Documentation/CodingGuidelines b/Documentation/CodingGuidelines\nindex 1e0c4cf..d72e912 100644\n--- a/Documentation/CodingGuidelines\n+++ b/Documentation/CodingGuidelines\n@@ -97,6 +97,17 @@ For shell scripts specifically (not exhaustive):\n \"then\" should be on the next line for if statements, and \"do\"\n should be on the next line for \"while\" and \"for\".\n \n+\t(incorrect)\n+\tif test -f hello; then\n+\t\tdo this\n+\tfi\n+\n+\t(correct)\n+\tif test -f hello\n+\tthen\n+\t\tdo this\n+\tfi\n+\n - We prefer \"test\" over \"[ ... ]\".\n \n - We do not write the noiseword \"function\" in front of shell\n-- \n2.0.0-rc1-355-gd6d6511\n"},{"id":"240381","messageId":"1398894312-30763-6-git-send-email-gitster@pobox.com","threadId":"36558","inReplyTo":"1398894312-30763-1-git-send-email-gitster@pobox.com","subject":"[PATCH 5/8] CodingGuidelines: give an example for shell function preamble","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2014-04-30T21:45:09Z","receivedAt":"2014-04-30T21:45:09Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Signed-off-by: Junio C Hamano \n---\n Documentation/CodingGuidelines | 14 +++++++++++---\n 1 file changed, 11 insertions(+), 3 deletions(-)\n\ndiff --git a/Documentation/CodingGuidelines b/Documentation/CodingGuidelines\nindex d72e912..1b0cd2b 100644\n--- a/Documentation/CodingGuidelines\n+++ b/Documentation/CodingGuidelines\n@@ -113,9 +113,17 @@ For shell scripts specifically (not exhaustive):\n - We do not write the noiseword \"function\" in front of shell\n functions.\n \n- - We prefer a space between the function name and the parentheses. The\n- opening \"{\" should also be on the same line.\n- E.g.: my_function () {\n+ - We prefer a space between the function name and the parentheses,\n+ and no space inside the parentheses. The opening \"{\" should also\n+ be on the same line.\n+\n+\t(incorrect)\n+\tmy_function(){\n+\t\t...\n+\n+\t(correct)\n+\tmy_function () {\n+\t\t...\n \n - As to use of grep, stick to a subset of BRE (namely, no \\{m,n\\},\n [::], [==], or [..]) for portability.\n-- \n2.0.0-rc1-355-gd6d6511\n"},{"id":"240380","messageId":"1398894312-30763-7-git-send-email-gitster@pobox.com","threadId":"36558","inReplyTo":"1398894312-30763-1-git-send-email-gitster@pobox.com","subject":"[PATCH 6/8] CodingGuidelines: call the conditional statement \"if ()\", not \"if()\"","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2014-04-30T21:45:10Z","receivedAt":"2014-04-30T21:45:10Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"The point immediately before it is about having SP after the control\nkeyword. Follow it.\n\nSigned-off-by: Junio C Hamano \n---\n Documentation/CodingGuidelines | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/Documentation/CodingGuidelines b/Documentation/CodingGuidelines\nindex 1b0cd2b..21e4272 100644\n--- a/Documentation/CodingGuidelines\n+++ b/Documentation/CodingGuidelines\n@@ -184,7 +184,7 @@ For C programs:\n of \"else if\" statements, it can make sense to add braces to\n single line blocks.\n \n- - We try to avoid assignments inside if().\n+ - We try to avoid assignments inside \"if ()\" condition.\n \n - Try to make your code understandable. You may put comments\n in, but comments invariably tend to stale out when the code\n-- \n2.0.0-rc1-355-gd6d6511\n"},{"id":"240382","messageId":"1398894312-30763-8-git-send-email-gitster@pobox.com","threadId":"36558","inReplyTo":"1398894312-30763-1-git-send-email-gitster@pobox.com","subject":"[PATCH 7/8] CodingGuidelines: on comparison","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2014-04-30T21:45:11Z","receivedAt":"2014-04-30T21:45:11Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"See http://thread.gmane.org/gmane.comp.version-control.git/3903/focus=4126\n\nSigned-off-by: Junio C Hamano \n---\n Documentation/CodingGuidelines | 26 ++++++++++++++++++++++++++\n 1 file changed, 26 insertions(+)\n\ndiff --git a/Documentation/CodingGuidelines b/Documentation/CodingGuidelines\nindex 21e4272..86fb9f6 100644\n--- a/Documentation/CodingGuidelines\n+++ b/Documentation/CodingGuidelines\n@@ -212,6 +212,32 @@ For C programs:\n - Double negation is often harder to understand than no negation\n at all.\n \n+ - There are two schools of thought when it comes to comparison,\n+ especially inside a loop. Some people prefer to have less stable\n+ value on the left hand side and more stable value on the right hand\n+ side, e.g. if you have a loop that counts variable i down to the\n+ lower bound,\n+\n+\twhile (i > lower_bound) {\n+\t\tdo something;\n+\t\ti--;\n+\t}\n+\n+ Other people prefer to have the textual order of values match the\n+ actual order of values in their comparison, so that they can\n+ mentally draw a number line from left to right and place these\n+ values in order, i.e.\n+\n+\twhile (lower_bound < i) {\n+\t\tdo something;\n+\t\ti--;\n+\t}\n+\n+ Both are valid, and we use both, even though we tend to see the\n+ former the more preferable, the more \"stable\" the more stable side\n+ becomes (comparison with a constant, \"i > 0\", is an extreme\n+ example). Just do not mix styles in the same part of the code.\n+\n - Some clever tricks, like using the !! operator with arithmetic\n constructs, can be extremely confusing to others. Avoid them,\n unless there is a compelling reason to use them.\n-- \n2.0.0-rc1-355-gd6d6511\n"},{"id":"240383","messageId":"1398894312-30763-9-git-send-email-gitster@pobox.com","threadId":"36558","inReplyTo":"1398894312-30763-1-git-send-email-gitster@pobox.com","subject":"[PATCH 8/8] CodingGuidelines: once it is in, it is not worth the code churn","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2014-04-30T21:45:12Z","receivedAt":"2014-04-30T21:45:12Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Signed-off-by: Junio C Hamano \n---\n Documentation/CodingGuidelines | 8 ++++++++\n 1 file changed, 8 insertions(+)\n\ndiff --git a/Documentation/CodingGuidelines b/Documentation/CodingGuidelines\nindex 86fb9f6..759003e 100644\n--- a/Documentation/CodingGuidelines\n+++ b/Documentation/CodingGuidelines\n@@ -18,6 +18,14 @@ code. For Git in general, three rough rules are:\n judgement call, the decision is based more on real world\n constraints people face than what the paper standard says.\n \n+ - Fixing style violations while working on a real change as a\n+ preparatory clean-up step is good, but otherwise avoid useless code\n+ churn for the sake of conforming to the style.\n+\n+ \"Once it _is_ in the tree, it's not really worth the patch noise to\n+ go and fix it up.\"\n+ Cf. http://article.gmane.org/gmane.linux.kernel/943020\n+\n Make your code readable and sensible, and don't try to be clever.\n \n As for more concrete guidelines, just imitate the existing code\n-- \n2.0.0-rc1-355-gd6d6511\n"},{"id":"240385","messageId":"5361711F.1030208@gmail.com","threadId":"36558","inReplyTo":"1398894312-30763-5-git-send-email-gitster@pobox.com","subject":"Re: [PATCH 4/8] CodingGuidelines: give an example for control statements","fromName":"Stefan Beller","fromEmail":"stefanbeller@gmail.com","sentAt":"2014-04-30T21:54:39Z","receivedAt":"2014-04-30T21:54:39Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"On 30.04.2014 23:45, Junio C Hamano wrote:\n> Signed-off-by: Junio C Hamano \n> ---\n> Documentation/CodingGuidelines | 11 +++++++++++\n> 1 file changed, 11 insertions(+)\n> \n> diff --git a/Documentation/CodingGuidelines b/Documentation/CodingGuidelines\n> index 1e0c4cf..d72e912 100644\n> --- a/Documentation/CodingGuidelines\n> +++ b/Documentation/CodingGuidelines\n> @@ -97,6 +97,17 @@ For shell scripts specifically (not exhaustive):\n> \"then\" should be on the next line for if statements, and \"do\"\n> should be on the next line for \"while\" and \"for\".\n> \n> +\t(incorrect)\n\nAt the other patches you used #comments behind oneliners,\nnot sure if that's also suitable here for consistency of the documentation.\n\n> +\tif test -f hello; then\n> +\t\tdo this\n> +\tfi\n> +\n> +\t(correct)\n> +\tif test -f hello\n> +\tthen\n> +\t\tdo this\n> +\tfi\n> +\n> - We prefer \"test\" over \"[ ... ]\".\n> \n> - We do not write the noiseword \"function\" in front of shell\n> \n"},{"id":"240386","messageId":"xmqqwqe6cw5u.fsf@gitster.dls.corp.google.com","threadId":"36558","inReplyTo":"5361711F.1030208@gmail.com","subject":"Re: [PATCH 4/8] CodingGuidelines: give an example for control statements","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2014-04-30T22:03:25Z","receivedAt":"2014-04-30T22:03:25Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Stefan Beller writes:\n\n> On 30.04.2014 23:45, Junio C Hamano wrote:\n>> Signed-off-by: Junio C Hamano \n>> ---\n>> Documentation/CodingGuidelines | 11 +++++++++++\n>> 1 file changed, 11 insertions(+)\n>> \n>> diff --git a/Documentation/CodingGuidelines b/Documentation/CodingGuidelines\n>> index 1e0c4cf..d72e912 100644\n>> --- a/Documentation/CodingGuidelines\n>> +++ b/Documentation/CodingGuidelines\n>> @@ -97,6 +97,17 @@ For shell scripts specifically (not exhaustive):\n>> \"then\" should be on the next line for if statements, and \"do\"\n>> should be on the next line for \"while\" and \"for\".\n>> \n>> +\t(incorrect)\n>\n> At the other patches you used #comments behind oneliners,\n> not sure if that's also suitable here for consistency of the documentation.\n\nI think you meant this one (are there others?):\n\n+\tcat hello > world < universe\t# incorrect\n+\tcat hello >world $world\t\t# incorrect\n+\techo hello >\"$world\"\t\t# correct\n+\n\nI can certainly go with this instead over there:\n\n\t(incorrect)\n\tcat hello > world < universe\n\techo hello >$world\n\n\t(correct)\n\tcat hello >world \"$world\"\n\nI do not think it is wise to use the trailing comment style on all\nthe incorrect/correct lines for these examples.\n\n>> +\tif test -f hello; then\n>> +\t\tdo this\n>> +\tfi\n>> +\n>> +\t(correct)\n>> +\tif test -f hello\n>> +\tthen\n>> +\t\tdo this\n>> +\tfi\n>> +\n>> - We prefer \"test\" over \"[ ... ]\".\n>> \n>> - We do not write the noiseword \"function\" in front of shell\n>> \n"},{"id":"240388","messageId":"xmqqsioucvo0.fsf_-_@gitster.dls.corp.google.com","threadId":"36558","inReplyTo":"1398894312-30763-4-git-send-email-gitster@pobox.com","subject":"[PATCH v2 3/8] CodingGuidelines: give an example for redirection","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2014-04-30T22:14:07Z","receivedAt":"2014-04-30T22:14:07Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\nSigned-off-by: Junio C Hamano \n---\n\n * Instead of using trailing \"# correct\" comment on each of the\n one-liners, group the good ones and the bad ones into two groups\n and give (incorrect)/(correct) header like the other patches as\n suggested. How does this look?\n\n The only reason I originally did these differently is because\n these are two independent examples, and I thought it would make\n the difference stand out better to the readers if we showed\n good/bad pairs of \"cat\" and \"echo\" that way. But as long as we\n are not going to add more examples here, I think the result is\n not too bad.\n\n Documentation/CodingGuidelines | 8 ++++++++\n 1 file changed, 8 insertions(+)\n\ndiff --git a/Documentation/CodingGuidelines b/Documentation/CodingGuidelines\nindex 6a8e94b..6bfe96e 100644\n--- a/Documentation/CodingGuidelines\n+++ b/Documentation/CodingGuidelines\n@@ -61,6 +61,14 @@ For shell scripts specifically (not exhaustive):\n redirection target in a variable (as shown above), our code does so\n because some versions of bash issue a warning without the quotes.\n \n+\t(incorrect)\n+\tcat hello > world < universe\n+\techo hello >$world\n+\n+\t(correct)\n+\tcat hello >world \"$world\"\n+\n - We prefer $( ... ) for command substitution; unlike ``, it\n properly nests. It should have been the way Bourne spelled\n it from day one, but unfortunately isn't.\n-- \n2.0.0-rc1-355-gd6d6511\n"},{"id":"240402","messageId":"87tx99zj31.fsf@fencepost.gnu.org","threadId":"36558","inReplyTo":"1398894312-30763-2-git-send-email-gitster@pobox.com","subject":"Re: [PATCH 1/8] CodingGuidelines: typofix","fromName":"David Kastrup","fromEmail":"dak@gnu.org","sentAt":"2014-05-01T14:09:38Z","receivedAt":"2014-05-01T14:09:38Z","isPatch":true,"sender":{"key":"dak@gnu.org","avatar":"https://avatars.githubusercontent.com/u/52141349?v=4"},"body":"Junio C Hamano writes:\n\n> The sentence lacked the necessary verb.\n\nNo, it didn't.\n\n> Signed-off-by: Junio C Hamano \n> ---\n> Documentation/CodingGuidelines | 2 +-\n> 1 file changed, 1 insertion(+), 1 deletion(-)\n>\n> diff --git a/Documentation/CodingGuidelines b/Documentation/CodingGuidelines\n> index f424dbd..fdf6269 100644\n> --- a/Documentation/CodingGuidelines\n> +++ b/Documentation/CodingGuidelines\n> @@ -15,7 +15,7 @@ code. For Git in general, three rough rules are:\n> let's use it\".\n> \n> Again, we live in the real world, and it is sometimes a\n ^^\n> - judgement call, the decision based more on real world\n> + judgement call, the decision is based more on real world\n> constraints people face than what the paper standard says.\n\nThere is one common \"is\" for original statement and paraphrase. Adding\nanother one turns this into two sentences which cannot sensibly be\nconnected with a comma.\n\nIf you want to fix something here, do s/judgement/judgment/ instead.\n\n-- \nDavid Kastrup\n"},{"id":"240404","messageId":"87ppjxziyo.fsf@fencepost.gnu.org","threadId":"36558","inReplyTo":"1398894312-30763-5-git-send-email-gitster@pobox.com","subject":"Re: [PATCH 4/8] CodingGuidelines: give an example for control statements","fromName":"David Kastrup","fromEmail":"dak@gnu.org","sentAt":"2014-05-01T14:12:15Z","receivedAt":"2014-05-01T14:12:15Z","isPatch":true,"sender":{"key":"dak@gnu.org","avatar":"https://avatars.githubusercontent.com/u/52141349?v=4"},"body":"Junio C Hamano writes:\n\n> Signed-off-by: Junio C Hamano \n> ---\n> Documentation/CodingGuidelines | 11 +++++++++++\n> 1 file changed, 11 insertions(+)\n>\n> diff --git a/Documentation/CodingGuidelines b/Documentation/CodingGuidelines\n> index 1e0c4cf..d72e912 100644\n> --- a/Documentation/CodingGuidelines\n> +++ b/Documentation/CodingGuidelines\n> @@ -97,6 +97,17 @@ For shell scripts specifically (not exhaustive):\n> \"then\" should be on the next line for if statements, and \"do\"\n> should be on the next line for \"while\" and \"for\".\n> \n> +\t(incorrect)\n> +\tif test -f hello; then\n> +\t\tdo this\n> +\tfi\n> +\n> +\t(correct)\n> +\tif test -f hello\n> +\tthen\n> +\t\tdo this\n> +\tfi\n> +\n> - We prefer \"test\" over \"[ ... ]\".\n> \n> - We do not write the noiseword \"function\" in front of shell\n\ns/noiseword/bashism/\n\n-- \nDavid Kastrup\n"},{"id":"240403","messageId":"87lhulziv6.fsf@fencepost.gnu.org","threadId":"36558","inReplyTo":"1398894312-30763-7-git-send-email-gitster@pobox.com","subject":"Re: [PATCH 6/8] CodingGuidelines: call the conditional statement \"if ()\", not \"if()\"","fromName":"David Kastrup","fromEmail":"dak@gnu.org","sentAt":"2014-05-01T14:14:21Z","receivedAt":"2014-05-01T14:14:21Z","isPatch":true,"sender":{"key":"dak@gnu.org","avatar":"https://avatars.githubusercontent.com/u/52141349?v=4"},"body":"Junio C Hamano writes:\n\n> The point immediately before it is about having SP after the control\n> keyword. Follow it.\n>\n> Signed-off-by: Junio C Hamano \n> ---\n> Documentation/CodingGuidelines | 2 +-\n> 1 file changed, 1 insertion(+), 1 deletion(-)\n>\n> diff --git a/Documentation/CodingGuidelines b/Documentation/CodingGuidelines\n> index 1b0cd2b..21e4272 100644\n> --- a/Documentation/CodingGuidelines\n> +++ b/Documentation/CodingGuidelines\n> @@ -184,7 +184,7 @@ For C programs:\n> of \"else if\" statements, it can make sense to add braces to\n> single line blocks.\n> \n> - - We try to avoid assignments inside if().\n> + - We try to avoid assignments inside \"if ()\" condition.\n\nThat's not grammatical.\n\n-- \nDavid Kastrup\n"},{"id":"240408","messageId":"xmqqoazhcrqg.fsf@gitster.dls.corp.google.com","threadId":"36558","inReplyTo":"87tx99zj31.fsf@fencepost.gnu.org","subject":"Re: [PATCH 1/8] CodingGuidelines: typofix","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2014-05-01T17:51:19Z","receivedAt":"2014-05-01T17:51:19Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"David Kastrup writes:\n\n> Junio C Hamano writes:\n>\n>> The sentence lacked the necessary verb.\n>\n> No, it didn't.\n>\n>> Signed-off-by: Junio C Hamano \n>> ---\n>> Documentation/CodingGuidelines | 2 +-\n>> 1 file changed, 1 insertion(+), 1 deletion(-)\n>>\n>> diff --git a/Documentation/CodingGuidelines b/Documentation/CodingGuidelines\n>> index f424dbd..fdf6269 100644\n>> --- a/Documentation/CodingGuidelines\n>> +++ b/Documentation/CodingGuidelines\n>> @@ -15,7 +15,7 @@ code. For Git in general, three rough rules are:\n>> let's use it\".\n>> \n>> Again, we live in the real world, and it is sometimes a\n> ^^\n>> - judgement call, the decision based more on real world\n>> + judgement call, the decision is based more on real world\n>> constraints people face than what the paper standard says.\n>\n> There is one common \"is\" for original statement and paraphrase. Adding\n> another one turns this into two sentences which cannot sensibly be\n> connected with a comma.\n\nThanks for spotting.\n\nI thought (but I see I didn't by mistake) that I split them into two\nsentences, replacing the comma with a semicolon.\n\n> If you want to fix something here, do s/judgement/judgment/ instead.\n\nThat too.\n"},{"id":"240410","messageId":"xmqqk3a5crb3.fsf@gitster.dls.corp.google.com","threadId":"36558","inReplyTo":"87ppjxziyo.fsf@fencepost.gnu.org","subject":"Re: [PATCH 4/8] CodingGuidelines: give an example for control statements","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2014-05-01T18:00:32Z","receivedAt":"2014-05-01T18:00:32Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"David Kastrup writes:\n\n>> - We do not write the noiseword \"function\" in front of shell\n>\n> s/noiseword/bashism/\n\nThat is outside the scope of this patch, but since you brought it\nup...\n\nI did consider between noiseword and bashism when I wrote this part,\nand decided against \"bashism\". XCU 2.4 \"Reserved Words\" lists it\n(among others) and says\n\n ... may be recognized as reserved words on some implementations\n ... causing unspecified results\n\nEven if \"bash\" were not the only shell that uses \"function\" keyword\nto introduce a shell function definition, we wouldn't use it. As we\nsay in the introductory part, we may say \"It is not in POSIX, but it\nis supported so widely and using it give us so great a benefit, so\nwe do use it\" for some things, but \"function\" is not one of them.\n\nThe reason is because it is a noiseword and its use is not necessary\nin order to define a shell function.\n"},{"id":"240413","messageId":"xmqqfvktcqsx.fsf@gitster.dls.corp.google.com","threadId":"36558","inReplyTo":"87lhulziv6.fsf@fencepost.gnu.org","subject":"Re: [PATCH 6/8] CodingGuidelines: call the conditional statement \"if ()\", not \"if()\"","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2014-05-01T18:11:26Z","receivedAt":"2014-05-01T18:11:26Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"David Kastrup writes:\n\n>> - - We try to avoid assignments inside if().\n>> + - We try to avoid assignments inside \"if ()\" condition.\n>\n> That's not grammatical.\n\nOK,\n\n\t... inside the condition part of an \"if ()\" statement.\n\nthen?\n"},{"id":"240415","messageId":"87d2fxz6py.fsf@fencepost.gnu.org","threadId":"36558","inReplyTo":"xmqqfvktcqsx.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH 6/8] CodingGuidelines: call the conditional statement \"if ()\", not \"if()\"","fromName":"David Kastrup","fromEmail":"dak@gnu.org","sentAt":"2014-05-01T18:36:41Z","receivedAt":"2014-05-01T18:36:41Z","isPatch":true,"sender":{"key":"dak@gnu.org","avatar":"https://avatars.githubusercontent.com/u/52141349?v=4"},"body":"Junio C Hamano writes:\n\n> David Kastrup writes:\n>\n>>> - - We try to avoid assignments inside if().\n>>> + - We try to avoid assignments inside \"if ()\" condition.\n>>\n>> That's not grammatical.\n>\n> OK,\n>\n> \t... inside the condition part of an \"if ()\" statement.\n>\n> then?\n\n ... in the condition of an \"if\" statement\n\nis what I would write. \"if ()\" statement is mixing name and shorthand\nfor the statement.\n\n-- \nDavid Kastrup\n"},{"id":"240476","messageId":"20140501212714.GB14441@sigill.intra.peff.net","threadId":"36558","inReplyTo":"xmqqoazhcrqg.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH 1/8] CodingGuidelines: typofix","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2014-05-01T21:27:14Z","receivedAt":"2014-05-01T21:27:14Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, May 01, 2014 at 10:51:19AM -0700, Junio C Hamano wrote:\n\n> > If you want to fix something here, do s/judgement/judgment/ instead.\n> \n> That too.\n\nFWIW, neither is outright wrong; it is an America/British variation, and\napparently dictionaries disagree on which is preferred.\n\n-Peff\n"},{"id":"240477","messageId":"20140501213657.GC14441@sigill.intra.peff.net","threadId":"36558","inReplyTo":"1398894312-30763-8-git-send-email-gitster@pobox.com","subject":"Re: [PATCH 7/8] CodingGuidelines: on comparison","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2014-05-01T21:36:57Z","receivedAt":"2014-05-01T21:36:57Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, Apr 30, 2014 at 02:45:11PM -0700, Junio C Hamano wrote:\n\n> See http://thread.gmane.org/gmane.comp.version-control.git/3903/focus=4126\n> \n> Signed-off-by: Junio C Hamano \n\nDon't you often complain about submitters referencing a discussion\nin a commit message without providing some context or summary?\n\n> + - There are two schools of thought when it comes to comparison,\n> + especially inside a loop. Some people prefer to have less stable\n> + value on the left hand side and more stable value on the right hand\n> + side, e.g. if you have a loop that counts variable i down to the\n> + lower bound,\n\nGrammar: /(less|more) stable/the &/\n\n> + Both are valid, and we use both, even though we tend to see the\n> + former the more preferable, the more \"stable\" the more stable side\n> + becomes (comparison with a constant, \"i > 0\", is an extreme\n> + example). Just do not mix styles in the same part of the code.\n> +\n\nI had trouble parsing the first sentence. Maybe:\n\n Both are valid, and we use both. However, the more \"stable\" the stable\n side becomes, the more we tend to prefer the former (comparison with a\n constant[...]\n\n-Peff\n"},{"id":"240540","messageId":"xmqqk3a4avt1.fsf@gitster.dls.corp.google.com","threadId":"36558","inReplyTo":"20140501213657.GC14441@sigill.intra.peff.net","subject":"Re: [PATCH 7/8] CodingGuidelines: on comparison","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2014-05-02T18:18:34Z","receivedAt":"2014-05-02T18:18:34Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King writes:\n\n> On Wed, Apr 30, 2014 at 02:45:11PM -0700, Junio C Hamano wrote:\n>\n>> See http://thread.gmane.org/gmane.comp.version-control.git/3903/focus=4126\n>> \n>> Signed-off-by: Junio C Hamano \n>\n> Don't you often complain about submitters referencing a discussion\n> in a commit message without providing some context or summary?\n\nYes, but the summary of the discussion would be identical to the new\ntext added by the patch to the documentation tree in this case, so I\ndidn't find a good introductory text before \"See $URL\". Perhaps\n\n This comes up from time to time. See $URL for the original\n discussion.\n\nbut I do not know if that is much better.\n\n>> + - There are two schools of thought when it comes to comparison,\n>> + especially inside a loop. Some people prefer to have less stable\n>> + value on the left hand side and more stable value on the right hand\n>> + side, e.g. if you have a loop that counts variable i down to the\n>> + lower bound,\n>\n> Grammar: /(less|more) stable/the &/\n>\n>> + Both are valid, and we use both, even though we tend to see the\n>> + former the more preferable, the more \"stable\" the more stable side\n>> + becomes (comparison with a constant, \"i > 0\", is an extreme\n>> + example). Just do not mix styles in the same part of the code.\n>> +\n>\n> I had trouble parsing the first sentence. Maybe:\n>\n> Both are valid, and we use both. However, the more \"stable\" the stable\n> side becomes, the more we tend to prefer the former (comparison with a\n> constant[...]\n\nThanks. That is much better.\n"},{"id":"240542","messageId":"xmqqfvksav81.fsf@gitster.dls.corp.google.com","threadId":"36558","inReplyTo":"20140501212714.GB14441@sigill.intra.peff.net","subject":"Re: [PATCH 1/8] CodingGuidelines: typofix","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2014-05-02T18:31:10Z","receivedAt":"2014-05-02T18:31:10Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King writes:\n\n> On Thu, May 01, 2014 at 10:51:19AM -0700, Junio C Hamano wrote:\n>\n>> > If you want to fix something here, do s/judgement/judgment/ instead.\n>> \n>> That too.\n>\n> FWIW, neither is outright wrong; it is an America/British variation, and\n> apparently dictionaries disagree on which is preferred.\n\nMy reading of various \"grammar\" sites was that even though variation\nexists[*1*], the form without 'e' is the traditionally preferred\nform, and that is why I said \"That too\".\n\nBut let's follow this one:\n\nhttp://www.google.com/trends/explore#q=judgement%20call%2C%20judgment%20call&cmpt=q\n\nwhich seems to say that with 'e' is more common.\n\n\n[Footnote]\n\n*1* To Americans, the form with 'e' is abomination. Wikipedia\nclaims that (1) without 'e' is in legal and (2) with 'e' in other\ncontexts in British (this particular one is a non-legal use), and\n(3) both are equally acceptable in non-legal contexts in Austraria\nand Canada.\n"},{"id":"240559","messageId":"20140502203116.GA32500@sigill.intra.peff.net","threadId":"36558","inReplyTo":"xmqqk3a4avt1.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH 7/8] CodingGuidelines: on comparison","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2014-05-02T20:31:16Z","receivedAt":"2014-05-02T20:31:16Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Fri, May 02, 2014 at 11:18:34AM -0700, Junio C Hamano wrote:\n\n> Jeff King writes:\n> \n> > On Wed, Apr 30, 2014 at 02:45:11PM -0700, Junio C Hamano wrote:\n> >\n> >> See http://thread.gmane.org/gmane.comp.version-control.git/3903/focus=4126\n> >> \n> >> Signed-off-by: Junio C Hamano \n> >\n> > Don't you often complain about submitters referencing a discussion\n> > in a commit message without providing some context or summary?\n> \n> Yes, but the summary of the discussion would be identical to the new\n> text added by the patch to the documentation tree in this case, so I\n> didn't find a good introductory text before \"See $URL\". Perhaps\n> \n> This comes up from time to time. See $URL for the original\n> discussion.\n> \n> but I do not know if that is much better.\n\nI meant something even less in-depth. Your message says only \"on\ncomparison\", and I did not even know what \"this\" in your sentence above\nwould mean until I followed the link.\n\n There are arguments for writing a conditional as \"a < b\" rather than\n \"b > a\", or vice versa. Let's give guidance on which we prefer.\n\nNot a big deal, but I think it is easy when you have just written the\npatch to forget that a reviewer or a reader of \"git log\" six months have\nno has no context at all.\n\n-Peff\n"},{"id":"240560","messageId":"20140502203327.GB32500@sigill.intra.peff.net","threadId":"36558","inReplyTo":"xmqqfvksav81.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH 1/8] CodingGuidelines: typofix","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2014-05-02T20:33:27Z","receivedAt":"2014-05-02T20:33:27Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Fri, May 02, 2014 at 11:31:10AM -0700, Junio C Hamano wrote:\n\n> But let's follow this one:\n> \n> http://www.google.com/trends/explore#q=judgement%20call%2C%20judgment%20call&cmpt=q\n> \n> which seems to say that with 'e' is more common.\n\nGrammar by democracy. ;)\n\n> *1* To Americans, the form with 'e' is abomination. Wikipedia\n> claims that (1) without 'e' is in legal and (2) with 'e' in other\n> contexts in British (this particular one is a non-legal use), and\n> (3) both are equally acceptable in non-legal contexts in Austraria\n> and Canada.\n\nThat is what I found most interesting about the discussion. The reason I\nbothered to look it up and say something is that as an American, I would\nwithout a doubt spell it with the \"e\", contradicting what I found\nonline. Oh well. \n\n-Peff\n"},{"id":"240565","messageId":"53640220eeca1_135215292ec1b@nysa.notmuch","threadId":"36558","inReplyTo":"20140502203327.GB32500@sigill.intra.peff.net","subject":"Re: [PATCH 1/8] CodingGuidelines: typofix","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2014-05-02T20:37:52Z","receivedAt":"2014-05-02T20:37:52Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"Jeff King wrote:\n> On Fri, May 02, 2014 at 11:31:10AM -0700, Junio C Hamano wrote:\n> \n> > But let's follow this one:\n> > \n> > http://www.google.com/trends/explore#q=judgement%20call%2C%20judgment%20call&cmpt=q\n> > \n> > which seems to say that with 'e' is more common.\n> \n> Grammar by democracy. ;)\n\nLanguages are a democracy. There's no authority that decides if\n\"unibrow\" should become part of the English language. We all do.\n\n-- \nFelipe Contreras\n"},{"id":"240563","messageId":"xmqqppjvap0a.fsf@gitster.dls.corp.google.com","threadId":"36558","inReplyTo":"20140502203116.GA32500@sigill.intra.peff.net","subject":"Re: [PATCH 7/8] CodingGuidelines: on comparison","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2014-05-02T20:45:25Z","receivedAt":"2014-05-02T20:45:25Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King writes:\n\n> I meant something even less in-depth. Your message says only \"on\n> comparison\", and I did not even know what \"this\" in your sentence above\n> would mean until I followed the link.\n>\n> There are arguments for writing a conditional as \"a < b\" rather than\n> \"b > a\", or vice versa. Let's give guidance on which we prefer.\n>\n> Not a big deal, but I think it is easy when you have just written the\n> patch to forget that a reviewer or a reader of \"git log\" six months have\n> no has no context at all.\n\nThanks; I'll steal that one.\n"},{"id":"240566","messageId":"xmqqiopnaopg.fsf@gitster.dls.corp.google.com","threadId":"36558","inReplyTo":"1398894312-30763-1-git-send-email-gitster@pobox.com","subject":"[PATCH 9/8] CodingGuidelines: on splitting a long line","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2014-05-02T20:51:55Z","receivedAt":"2014-05-02T20:51:55Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\nSigned-off-by: Junio C Hamano \n---\n Documentation/CodingGuidelines | 55 ++++++++++++++++++++++++++++++++++++++++++\n 1 file changed, 55 insertions(+)\n\ndiff --git a/Documentation/CodingGuidelines b/Documentation/CodingGuidelines\nindex 02ca67c..4dd07c3 100644\n--- a/Documentation/CodingGuidelines\n+++ b/Documentation/CodingGuidelines\n@@ -249,6 +249,61 @@ For C programs:\n Just do not mix styles in the same part of the code and mimic\n existing styles in the neighbourhood.\n \n+ - There are two schools of thought when it comes to splitting a long\n+ logical line into multiple lines. Some people push the second and\n+ subsequent lines far enough to the right with tabs and align them:\n+\n+ if (the_beginning_of_a_very_long_expression_that_has_to ||\n+\t\tspan_more_than_a_single_line_of ||\n+\t\tthe_source_text) {\n+ ...\n+\n+ while other people prefer to align the second and the subsequent\n+ lines with the column immediately inside the opening parenthesis,\n+ with tabs and spaces, following our \"tabstop is always a multiple\n+ of 8\" convention:\n+\n+ if (the_beginning_of_a_very_long_expression_that_has_to ||\n+\t span_more_than_a_single_line_of ||\n+\t the_source_text) {\n+ ...\n+\n+ Both are valid, and we use both. Again, just do not mix styles in\n+ the same part of the code and mimic existing styles in the\n+ neighbourhood.\n+\n+ - When splitting a long logical line, some people change line before\n+ a binary operator, so that the result looks like a parse tree when\n+ you turn your head 90-degrees counterclockwise:\n+\n+ if (the_beginning_of_a_very_long_expression_that_has_to\n+\t || span_more_than_a_single_line_of_the_source_text) {\n+\n+ while other people prefer to leave the operator at the end of the\n+ line:\n+\n+ if (the_beginning_of_a_very_long_expression_that_has_to ||\n+\t span_more_than_a_single_line_of_the_source_text) {\n+\n+ Both are valid, but we tend to use the latter more, unless the\n+ expression gets fairly complex, in which case the former tends to\n+ be easier to read. Again, just do not mix styles in the same part\n+ of the code and mimic existing styles in the neighbourhood.\n+\n+ - When splitting a long logical line, with everything else being\n+ equal, it is preferrable to split after the operator at higher\n+ level in the parse tree. That is, this is more preferrable:\n+\n+\tif (a_very_long_variable * that_is_used_in +\n+\t a_very_long_expression) {\n+\t\t...\n+\n+ than\n+\n+\tif (a_very_long_variable *\n+\t that_is_used_in + a_very_long_expression) {\n+\t\t...\n+\n - Some clever tricks, like using the !! operator with arithmetic\n constructs, can be extremely confusing to others. Avoid them,\n unless there is a compelling reason to use them.\n-- \n2.0.0-rc1-355-gd6d6511\n"},{"id":"240567","messageId":"871twbykah.fsf@fencepost.gnu.org","threadId":"36558","inReplyTo":"53640220eeca1_135215292ec1b@nysa.notmuch","subject":"Re: [PATCH 1/8] CodingGuidelines: typofix","fromName":"David Kastrup","fromEmail":"dak@gnu.org","sentAt":"2014-05-02T20:53:26Z","receivedAt":"2014-05-02T20:53:26Z","isPatch":true,"sender":{"key":"dak@gnu.org","avatar":"https://avatars.githubusercontent.com/u/52141349?v=4"},"body":"Felipe Contreras writes:\n\n> Jeff King wrote:\n>> On Fri, May 02, 2014 at 11:31:10AM -0700, Junio C Hamano wrote:\n>> \n>> > But let's follow this one:\n>> > \n>> > http://www.google.com/trends/explore#q=judgement%20call%2C%20judgment%20call&cmpt=q\n>> > \n>> > which seems to say that with 'e' is more common.\n>> \n>> Grammar by democracy. ;)\n>\n> Languages are a democracy. There's no authority that decides if\n> \"unibrow\" should become part of the English language. We all do.\n\nWell, and the U.S. justice system rather supports the hyphenation judge-\nmental.\n\n-- \nDavid Kastrup\n"},{"id":"240570","messageId":"20140502210042.GH75770@vauxhall.crustytoothpaste.net","threadId":"36558","inReplyTo":"xmqqiopnaopg.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH 9/8] CodingGuidelines: on splitting a long line","fromName":"brian m. carlson","fromEmail":"sandals@crustytoothpaste.net","sentAt":"2014-05-02T21:00:42Z","receivedAt":"2014-05-02T21:00:42Z","isPatch":true,"sender":{"key":"sandals@crustytoothpaste.net","avatar":"https://avatars.githubusercontent.com/u/497054?v=4"},"body":"On Fri, May 02, 2014 at 01:51:55PM -0700, Junio C Hamano wrote:\n> + - When splitting a long logical line, with everything else being\n> + equal, it is preferrable to split after the operator at higher\n\ndict.org says that it should be \"preferable\", not \"preferrable\".\n\n-- \nbrian m. carlson / brian with sandals: Houston, Texas, US\n+1 832 623 2791 | http://www.crustytoothpaste.net/~bmc | My opinion only\nOpenPGP: RSA v4 4096b: 88AC E9B2 9196 305B A994 7552 F1BA 225C 0223 B187\n"}]}