{"thread":{"id":"20994","subject":"[PATCH] Avoid the use of backslash-at-eol in pack-objects usage string.","startedAt":"2009-09-17T21:51:40Z","lastAt":"2009-09-18T21:40:47Z","messageCount":7,"participants":["Thiago Farina","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"123468","messageId":"1253224300-18017-1-git-send-email-tfransosi@gmail.com","threadId":"20994","inReplyTo":null,"subject":"[PATCH] Avoid the use of backslash-at-eol in pack-objects usage string.","fromName":"Thiago Farina","fromEmail":"tfransosi@gmail.com","sentAt":"2009-09-17T21:51:40Z","receivedAt":"2009-09-17T21:51:40Z","isPatch":true,"sender":{"key":"tfransosi@gmail.com","avatar":"https://avatars.githubusercontent.com/u/970071?v=4"},"body":"Signed-off-by: Thiago Farina \n---\n builtin-pack-objects.c | 18 +++++++++---------\n 1 files changed, 9 insertions(+), 9 deletions(-)\n\ndiff --git a/builtin-pack-objects.c b/builtin-pack-objects.c\nindex 7a390e1..4494a68 100644\n--- a/builtin-pack-objects.c\n+++ b/builtin-pack-objects.c\n@@ -22,15 +22,15 @@\n #include \n #endif\n \n-static const char pack_usage[] = \"\\\n-git pack-objects [{ -q | --progress | --all-progress }] \\n\\\n-\t[--max-pack-size=N] [--local] [--incremental] \\n\\\n-\t[--window=N] [--window-memory=N] [--depth=N] \\n\\\n-\t[--no-reuse-delta] [--no-reuse-object] [--delta-base-offset] \\n\\\n-\t[--threads=N] [--non-empty] [--revs [--unpacked | --all]*] [--reflog] \\n\\\n-\t[--stdout | base-name] [--include-tag] \\n\\\n-\t[--keep-unreachable | --unpack-unreachable] \\n\\\n-\t[ writes:\n\n> +static const char pack_usage[] =\n> + \"git pack-objects [{ -q | --progress | --all-progress }] \\n\"\n> + \" [--max-pack-size=N] [--local] [--incremental] \\n\"\n> + \" [--window=N] [--window-memory=N] [--depth=N] \\n\"\n> + \" [--no-reuse-delta] [--no-reuse-object] [--delta-base-offset] \\n\"\n> + \" [--threads=N] [--non-empty] [--revs [--unpacked | --all]*] [--reflog] \\n\"\n> + \" [--stdout | base-name] [--include-tag] \\n\"\n> + \" [--keep-unreachable | --unpack-unreachable] \\n\"\n> + \" [ wrote:\n> Thiago Farina writes:\n>\n>> +static const char pack_usage[] =\n>> +  \"git pack-objects [{ -q | --progress | --all-progress }] \\n\"\n>> +  \"        [--max-pack-size=N] [--local] [--incremental] \\n\"\n>> +  \"        [--window=N] [--window-memory=N] [--depth=N] \\n\"\n>> +  \"        [--no-reuse-delta] [--no-reuse-object] [--delta-base-offset] \\n\"\n>> +  \"        [--threads=N] [--non-empty] [--revs [--unpacked | --all]*] [--reflog] \\n\"\n>> +  \"        [--stdout | base-name] [--include-tag] \\n\"\n>> +  \"        [--keep-unreachable | --unpack-unreachable] \\n\"\n>> +  \"        [\n> Do you still want to keep the trailing whitespace on these lines?\nI did this to maintain the same output of the old string, but if you\nwant I can change, what you suggest?\n"},{"id":"123474","messageId":"7vd45pgjhr.fsf@alter.siamese.dyndns.org","threadId":"20994","inReplyTo":"a4c8a6d00909171506l6c4b6a49i22d7b337a0c6cfa2@mail.gmail.com","subject":"Re: [PATCH] Avoid the use of backslash-at-eol in pack-objects usage string.","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-09-18T00:54:40Z","receivedAt":"2009-09-18T00:54:40Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Thiago Farina writes:\n\n> On Thu, Sep 17, 2009 at 7:00 PM, Junio C Hamano wrote:\n>> Thiago Farina writes:\n>>\n>>> +static const char pack_usage[] =\n>>> + \"git pack-objects [{ -q | --progress | --all-progress }] \\n\"\n>>> + \" [--max-pack-size=N] [--local] [--incremental] \\n\"\n>>> + \" [--window=N] [--window-memory=N] [--depth=N] \\n\"\n>>> + \" [--no-reuse-delta] [--no-reuse-object] [--delta-base-offset] \\n\"\n>>> + \" [--threads=N] [--non-empty] [--revs [--unpacked | --all]*] [--reflog] \\n\"\n>>> + \" [--stdout | base-name] [--include-tag] \\n\"\n>>> + \" [--keep-unreachable | --unpack-unreachable] \\n\"\n>>> + \" [>\n>> Do you still want to keep the trailing whitespace on these lines?\n> I did this to maintain the same output of the old string, but if you\n> want I can change, what you suggest?\n\nIf you need to add or remove an option to actually _change_ the string, a\npatch like this, as a preparatory step before the real improvement, would\nbe a very welcome clean-up. I however would suggest doing nothing, if\nthis is the only patch you are going to send against this program in the\nnear future, to be honest.\n\nEven though we do not have any other patch in flight that changes this\nprogram at this moment (as expected, because we are in -rc freeze), which\nmeans there is not much risk for this patch to cause needless conflicts\nwith others, we generally avoid code churn like this one, as a principle\nfor a maturing project.\n\nThe _very best_ thing you can do for the project on this particular issue\nis to keep an eye on the list and the next time somebody wants to patch\nthis program in a way that affects the usage string, remind that person to\nfirst clean-up the string without changing anything else as a preparation\npatch; I however admit that I am asking a lot more work out of you.\n\nA real improvement patch from that somebody _could_ be to remove the\ntrailing whitespaces from the output string, and in that case I would not\nmind if two patches (one preparatory patch which is this one, and the\nother being the removal of trailing whitespaces) were squashed together.\nIn fact, in such a trivial case, it probably be better to squash them into\none.\n\nAnd that somebody _could_ be you.\n"},{"id":"123493","messageId":"a4c8a6d00909180802r713d7644mcb4e98ae4352a03a@mail.gmail.com","threadId":"20994","inReplyTo":"7vd45pgjhr.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH] Avoid the use of backslash-at-eol in pack-objects usage string.","fromName":"Thiago Farina","fromEmail":"tfransosi@gmail.com","sentAt":"2009-09-18T15:02:24Z","receivedAt":"2009-09-18T15:02:24Z","isPatch":true,"sender":{"key":"tfransosi@gmail.com","avatar":"https://avatars.githubusercontent.com/u/970071?v=4"},"body":"On Thu, Sep 17, 2009 at 9:54 PM, Junio C Hamano wrote:\n> Thiago Farina writes:\n>\n>> On Thu, Sep 17, 2009 at 7:00 PM, Junio C Hamano wrote:\n>>> Thiago Farina writes:\n>>>\n>>>> +static const char pack_usage[] =\n>>>> +  \"git pack-objects [{ -q | --progress | --all-progress }] \\n\"\n>>>> +  \"        [--max-pack-size=N] [--local] [--incremental] \\n\"\n>>>> +  \"        [--window=N] [--window-memory=N] [--depth=N] \\n\"\n>>>> +  \"        [--no-reuse-delta] [--no-reuse-object] [--delta-base-offset] \\n\"\n>>>> +  \"        [--threads=N] [--non-empty] [--revs [--unpacked | --all]*] [--reflog] \\n\"\n>>>> +  \"        [--stdout | base-name] [--include-tag] \\n\"\n>>>> +  \"        [--keep-unreachable | --unpack-unreachable] \\n\"\n>>>> +  \"        [>>\n>>> Do you still want to keep the trailing whitespace on these lines?\n>> I did this to maintain the same output of the old string, but if you\n>> want I can change, what you suggest?\n>\n> If you need to add or remove an option to actually _change_ the string, a\n> patch like this, as a preparatory step before the real improvement, would\n> be a very welcome clean-up.  I however would suggest doing nothing, if\n> this is the only patch you are going to send against this program in the\n> near future, to be honest.\nOK.\n>\n> Even though we do not have any other patch in flight that changes this\n> program at this moment (as expected, because we are in -rc freeze), which\n> means there is not much risk for this patch to cause needless conflicts\n> with others, we generally avoid code churn like this one, as a principle\n> for a maturing project.\nThis release candidate freeze is a period that no one can send patches?\nIf you could, point me to a documentation about how is the development\nprocess adopted by git.\nAs I can see, anybody can send patches to this mailing list for\nreview, but if no one cares about the my patch for example, it doesn't\nget a review and any feedback.\nIn a web review tool, the patch is assigned to someone review it, but\nhere it is impossible, so how the things are tracked here? Only you\nmerge the patches into the master branch? When someone send a patch,\nyou get it, make a topic-branch in your private repository, and if it\nis good it will be merged into 'pu branch'? And what did you mean with\ncode churn?\n>\n> The _very best_ thing you can do for the project on this particular issue\n> is to keep an eye on the list and the next time somebody wants to patch\n> this program in a way that affects the usage string, remind that person to\n> first clean-up the string without changing anything else as a preparation\n> patch; I however admit that I am asking a lot more work out of you.\n>\n> A real improvement patch from that somebody _could_ be to remove the\n> trailing whitespaces from the output string, and in that case I would not\n> mind if two patches (one preparatory patch which is this one, and the\n> other being the removal of trailing whitespaces) were squashed together.\n> In fact, in such a trivial case, it probably be better to squash them into\n> one.\nIf I understand correctly, do you want a function to remove the\ntrailing whitespace from a given string? Like the functions that work\nwith whitespaces in ws.c?\n>\n> And that somebody _could_ be you.\n>\n"},{"id":"123500","messageId":"7v1vm49ifb.fsf@alter.siamese.dyndns.org","threadId":"20994","inReplyTo":"a4c8a6d00909180802r713d7644mcb4e98ae4352a03a@mail.gmail.com","subject":"Re: [PATCH] Avoid the use of backslash-at-eol in pack-objects usage string.","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-09-18T19:11:52Z","receivedAt":"2009-09-18T19:11:52Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Thiago Farina writes:\n\n> This release candidate freeze is a period that no one can send patches?\n\nNo.\n\nAfter -rc1, only fixes to regressions and severe bugs and trivially\ncorrect documentation patches will be applied to my tree, but all other\nkinds of patches are still sent to the list for discussion, so that the\nproposed changes can be discussed, polished and then become ready for the\ndevelopment cycle after the upcoming release. I often even pick them up\nand queue them to 'pu' and possibly 'next' as time permits.\n\n> Only you\n> merge the patches into the master branch? When someone send a patch,\n> you get it, make a topic-branch in your private repository, and if it\n> is good it will be merged into 'pu branch'?\n\n\"A note from the maintainer\" I send out every once in a while (also can be\nseen at http://members.cox.net/junkio/) explains how things work.\n\n> And what did you mean with code churn?\n\nA change primarily for the sake of change without urgency nor real benefit\nin the longer term.\n\nIt bothers nobody if a long literal string is written as a string literal\nin a dq pair with LFs quoted with backslashes, or as a run of multiple\nstring literals, each of which ending with LF, to be concatenated by the\ncompiler. It however would bother somebody who actually wants to modify\nthese lines for a real change, and that is the best time for doing such a\nclean-up. Reasons for such a real change vary; to fix earlier mistakes\n(e.g. one line being excessively longer than others, or an option is\nmisspelled), to add a new option, or to make the output of the program\neasier to read in general, etc.\n\n>> A real improvement patch from that somebody _could_ be to remove the\n>> trailing whitespaces from the output string, and in that case I would not\n>> mind if two patches (one preparatory patch which is this one, and the\n>> other being the removal of trailing whitespaces) were squashed together.\n>> In fact, in such a trivial case, it probably be better to squash them into\n>> one.\n\n> If I understand correctly, do you want a function...\n\nNo.\n\nWhat I meant was that I might have said it is a real improvement if your\npatch also removed the trailing whitespace from the literal string, as I\nhinted in my original response.\n\nSuch a submission may have looked like this. Notice that the changing of\nthe style for multi-line string literal is \"while at it\". I called a\npatch whose only change falls into that category a needless code churn.\n\n-- >8 --\nSubject: [PATCH] pack-objects: remove SP at the end of usage string\nFrom: Thiago Farina \n\nThese spaces immediately before the end of lines are unnecessary.\n\nWhile at it, instead of using a single string literal with backslashes\nat end of each line, split the lines into individual string literals\nand tell the compiler to concatenate them.\n\nSigned-off-by: Thiago Farina \n---\n builtin-pack-objects.c | 18 +++++++++---------\n 1 files changed, 9 insertions(+), 9 deletions(-)\n\ndiff --git a/builtin-pack-objects.c b/builtin-pack-objects.c\nindex 7a390e1..02f9246 100644\n--- a/builtin-pack-objects.c\n+++ b/builtin-pack-objects.c\n@@ -22,15 +22,15 @@\n #include \n #endif\n \n-static const char pack_usage[] = \"\\\n-git pack-objects [{ -q | --progress | --all-progress }] \\n\\\n-\t[--max-pack-size=N] [--local] [--incremental] \\n\\\n-\t[--window=N] [--window-memory=N] [--depth=N] \\n\\\n-\t[--no-reuse-delta] [--no-reuse-object] [--delta-base-offset] \\n\\\n-\t[--threads=N] [--non-empty] [--revs [--unpacked | --all]*] [--reflog] \\n\\\n-\t[--stdout | base-name] [--include-tag] \\n\\\n-\t[--keep-unreachable | --unpack-unreachable] \\n\\\n-\t[ wrote:\n> Thiago Farina writes:\n>\n>> This release candidate freeze is a period that no one can send patches?\n>\n> No.\n>\n> After -rc1, only fixes to regressions and severe bugs and trivially\n> correct documentation patches will be applied to my tree, but all other\n> kinds of patches are still sent to the list for discussion, so that the\n> proposed changes can be discussed, polished and then become ready for the\n> development cycle after the upcoming release.\n> I often even pick them up and queue them to 'pu' and possibly 'next' as time permits.\nIf you don't want to pick a patch this means that it was not accepted,\nright? I wrote others two trivial patches, one has comments, and I did\nthe changes suggested, but I guess not will be done about it because\nit is just trivial.\n>> And what did you mean with code churn?\n>\n> A change primarily for the sake of change without urgency nor real benefit\n> in the longer term.\n>\n> It bothers nobody if a long literal string is written as a string literal\n> in a dq pair with LFs quoted with backslashes, or as a run of multiple\n> string literals, each of which ending with LF, to be concatenated by the\n> compiler.  It however would bother somebody who actually wants to modify\n> these lines for a real change, and that is the best time for doing such a\n> clean-up.  Reasons for such a real change vary; to fix earlier mistakes\n> (e.g. one line being excessively longer than others, or an option is\n> misspelled), to add a new option, or to make the output of the program\n> easier to read in general, etc.\nThis means that trivial patches like this one I wrote are generally\nnot accepted? Why is there this difficult (is it to maintain the high\nlevel of the patches)? I thought if it is trivial it can be merged\nafter a review, into one of the integration branches. You write the\ncomments (the people in mailing list), I make the changes, and then\nthe patch is committed. But what I'm seeing here, this is not how the\nthings are done here. It is much more complicated than that I guess.\n\nIn a codereview tool I can send a patch for review, I can assign it to\nsomeone review, he will make comments, I will make the necessary\nchanges, and when the patch is ready, it will be committed. What is\nthe workflow? With an email I can't assign a patch to someone, with\ntime it will be lost.\n\nI'm just trying to understand what I have to do, to submit better\npatches. Another issue that I saw, is about *issues* or bugs, they are\nnot tracked in a bug traker. It's just an email, so how can I work in\na bug if I don't know about it, have I to find the bugs myself?\n"}]}