# [PATCH 1/1] [OUTREACHY] Fixed add.c file to conform to guidelines when using die() listed in issue #635 7 messages from 2023-10-09 to 2023-10-11. Participants: Naomi Ibe, Junio C Hamano, Christian Couder, Dragan Simic. Thread: https://gitlist.dev/t/60328 ## Naomi Ibe, 2023-10-09 01:15 Subject: [PATCH 1/1] [OUTREACHY] Fixed add.c file to conform to guidelines when using die() listed in issue #635 Message-ID: <20231009011546.509-1-naomi.ibeh69@gmail.com> URL: https://gitlist.dev/e/20231009011546.509-1-naomi.ibeh69%40gmail.com ``` From: Naomi Signed-off-by: Naomi Ibe --- builtin/add.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/builtin/add.c b/builtin/add.c index c27254a5cd..5126d2ede3 100644 --- a/builtin/add.c +++ b/builtin/add.c @@ -182,7 +182,7 @@ static int edit_patch(int argc, const char **argv, const char *prefix) git_config(git_diff_basic_config, NULL); /* no "diff" UI options */ if (repo_read_index(the_repository) < 0) - die(_("Could not read the index")); + die(_("could not read the index")); repo_init_revisions(the_repository, &rev, prefix); rev.diffopt.context = 7; @@ -200,15 +200,15 @@ static int edit_patch(int argc, const char **argv, const char *prefix) die(_("editing patch failed")); if (stat(file, &st)) - die_errno(_("Could not stat '%s'"), file); + die_errno(_("could not stat '%s'"), file); if (!st.st_size) - die(_("Empty patch. Aborted.")); + die(_("empty patch. aborted")); child.git_cmd = 1; strvec_pushl(&child.args, "apply", "--recount", "--cached", file, NULL); if (run_command(&child)) - die(_("Could not apply '%s'"), file); + die(_("could not apply '%s'"), file); unlink(file); free(file); @@ -568,7 +568,7 @@ int cmd_add(int argc, const char **argv, const char *prefix) finish: if (write_locked_index(&the_index, &lock_file, COMMIT_LOCK | SKIP_IF_UNCHANGED)) - die(_("Unable to write new index file")); + die(_("unable to write new index file")); dir_clear(&dir); clear_pathspec(&pathspec); -- 2.36.1.windows.1 ``` ## Junio C Hamano, 2023-10-09 18:49 Subject: Re: [PATCH 1/1] [OUTREACHY] Fixed add.c file to conform to guidelines when using die() listed in issue #635 Message-ID: URL: https://gitlist.dev/e/xmqqlecbzl5e.fsf%40gitster.g In-Reply-To: <20231009011546.509-1-naomi.ibeh69@gmail.com> ``` Naomi Ibe writes: > Subject: Re: [PATCH 1/1] [OUTREACHY] Fixed add.c file to conform to guidelines when using d Subject: [OUTREACHY][PATCH 1/1] builtin/add.c: clean up die() messages > From: Naomi The name and address on this line come from your commit object, which in turn would have came from your configuration. You have [user] name = Naomi email = naomi.ibeh69@gmail.com somewhere in your configuration file, perhaps in $HOME/.gitconfig or somewhere. When contributiong to this project, you want that "name" line to also include your family name, as it should match what you write on your Signed-off-by: line. A focused way to do so without affecting your author identity for other projects is to add [user] name = Naomi Ibe in .git/config of the repository that you use to contribute to this project, e.g., $ cd ... to the working tree of your clone of git you work in ... $ git config user.name "Naomi Ibe" The space above your Sign off is to fill the details you omitted on the title of the message (which would say something about "conform to guidelines" or "clean up" without mentioning what rule the original violates), making the whole thing something like: builtin/add.c: clean up die() messages As described in the CodingGuidelines document, a single line message given to die() and its friends should not capitalize its first word, and should not add full-stop at the end. Signed-off-by: ... > Signed-off-by: Naomi Ibe > --- > builtin/add.c | 10 +++++----- > 1 file changed, 5 insertions(+), 5 deletions(-) Thanks. Otherwise the patch looks good. > > diff --git a/builtin/add.c b/builtin/add.c > index c27254a5cd..5126d2ede3 100644 > --- a/builtin/add.c > +++ b/builtin/add.c > @@ -182,7 +182,7 @@ static int edit_patch(int argc, const char **argv, const char *prefix) > git_config(git_diff_basic_config, NULL); /* no "diff" UI options */ > > if (repo_read_index(the_repository) < 0) > - die(_("Could not read the index")); > + die(_("could not read the index")); > > repo_init_revisions(the_repository, &rev, prefix); > rev.diffopt.context = 7; > @@ -200,15 +200,15 @@ static int edit_patch(int argc, const char **argv, const char *prefix) > die(_("editing patch failed")); > > if (stat(file, &st)) > - die_errno(_("Could not stat '%s'"), file); > + die_errno(_("could not stat '%s'"), file); > if (!st.st_size) > - die(_("Empty patch. Aborted.")); > + die(_("empty patch. aborted")); > > child.git_cmd = 1; > strvec_pushl(&child.args, "apply", "--recount", "--cached", file, > NULL); > if (run_command(&child)) > - die(_("Could not apply '%s'"), file); > + die(_("could not apply '%s'"), file); > > unlink(file); > free(file); > @@ -568,7 +568,7 @@ int cmd_add(int argc, const char **argv, const char *prefix) > finish: > if (write_locked_index(&the_index, &lock_file, > COMMIT_LOCK | SKIP_IF_UNCHANGED)) > - die(_("Unable to write new index file")); > + die(_("unable to write new index file")); > > dir_clear(&dir); > clear_pathspec(&pathspec); ``` ## Naomi Ibe, 2023-10-10 15:19 Subject: Re: [PATCH 1/1] [OUTREACHY] Fixed add.c file to conform to guidelines when using die() listed in issue #635 Message-ID: URL: https://gitlist.dev/e/CACS%3DG2yUGGJwD05KOFZK%2BAV3TSNDvDEfC%3DpFRsLwKX_-dgt%2BgA%40mail.gmail.com In-Reply-To: ``` Thank you very much! I'd definitely make those changes on my next patch. Should I begin work on version 2 or should I still wait for additional input on the version 1? On Mon, Oct 9, 2023 at 7:49 PM Junio C Hamano wrote: > > Naomi Ibe writes: > > > Subject: Re: [PATCH 1/1] [OUTREACHY] Fixed add.c file to conform to guidelines when using d > > Subject: [OUTREACHY][PATCH 1/1] builtin/add.c: clean up die() messages > > > From: Naomi > > The name and address on this line come from your commit object, > which in turn would have came from your configuration. You have > > [user] > name = Naomi > email = naomi.ibeh69@gmail.com > > somewhere in your configuration file, perhaps in $HOME/.gitconfig or > somewhere. When contributiong to this project, you want that "name" > line to also include your family name, as it should match what you > write on your Signed-off-by: line. A focused way to do so without > affecting your author identity for other projects is to add > > [user] > name = Naomi Ibe > > in .git/config of the repository that you use to contribute to this > project, e.g., > > $ cd ... to the working tree of your clone of git you work in ... > $ git config user.name "Naomi Ibe" > > The space above your Sign off is to fill the details you omitted on > the title of the message (which would say something about "conform > to guidelines" or "clean up" without mentioning what rule the > original violates), making the whole thing something like: > > builtin/add.c: clean up die() messages > > As described in the CodingGuidelines document, a single line > message given to die() and its friends should not capitalize its > first word, and should not add full-stop at the end. > > Signed-off-by: ... > > > Signed-off-by: Naomi Ibe > > --- > > builtin/add.c | 10 +++++----- > > 1 file changed, 5 insertions(+), 5 deletions(-) > > Thanks. Otherwise the patch looks good. > > > > > diff --git a/builtin/add.c b/builtin/add.c > > index c27254a5cd..5126d2ede3 100644 > > --- a/builtin/add.c > > +++ b/builtin/add.c > > @@ -182,7 +182,7 @@ static int edit_patch(int argc, const char **argv, const char *prefix) > > git_config(git_diff_basic_config, NULL); /* no "diff" UI options */ > > > > if (repo_read_index(the_repository) < 0) > > - die(_("Could not read the index")); > > + die(_("could not read the index")); > > > > repo_init_revisions(the_repository, &rev, prefix); > > rev.diffopt.context = 7; > > @@ -200,15 +200,15 @@ static int edit_patch(int argc, const char **argv, const char *prefix) > > die(_("editing patch failed")); > > > > if (stat(file, &st)) > > - die_errno(_("Could not stat '%s'"), file); > > + die_errno(_("could not stat '%s'"), file); > > if (!st.st_size) > > - die(_("Empty patch. Aborted.")); > > + die(_("empty patch. aborted")); > > > > child.git_cmd = 1; > > strvec_pushl(&child.args, "apply", "--recount", "--cached", file, > > NULL); > > if (run_command(&child)) > > - die(_("Could not apply '%s'"), file); > > + die(_("could not apply '%s'"), file); > > > > unlink(file); > > free(file); > > @@ -568,7 +568,7 @@ int cmd_add(int argc, const char **argv, const char *prefix) > > finish: > > if (write_locked_index(&the_index, &lock_file, > > COMMIT_LOCK | SKIP_IF_UNCHANGED)) > > - die(_("Unable to write new index file")); > > + die(_("unable to write new index file")); > > > > dir_clear(&dir); > > clear_pathspec(&pathspec); ``` ## Christian Couder, 2023-10-10 19:22 Subject: Re: [PATCH 1/1] [OUTREACHY] Fixed add.c file to conform to guidelines when using die() listed in issue #635 Message-ID: URL: https://gitlist.dev/e/CAP8UFD3R1bDcbE8V9xbVhoEv9JuVLLXBErp9c%3DCJVvTuBvyykA%40mail.gmail.com In-Reply-To: ``` On Tue, Oct 10, 2023 at 5:20 PM Naomi Ibe wrote: > > Thank you very much! I'd definitely make those changes on my next patch. > Should I begin work on version 2 or should I still wait for additional > input on the version 1? I think you can work on version 2. Also please reply inline instead of top-posting (see https://en.wikipedia.org/wiki/Posting_style). ``` ## Junio C Hamano, 2023-10-10 22:24 Subject: Re: [PATCH 1/1] [OUTREACHY] Fixed add.c file to conform to guidelines when using die() listed in issue #635 Message-ID: URL: https://gitlist.dev/e/xmqq34yiqfoh.fsf%40gitster.g In-Reply-To: ``` Naomi Ibe writes: > Thank you very much! I'd definitely make those changes on my next patch. [administrivia] do not top post. > Should I begin work on version 2 or should I still wait for additional > input on the version 1? There is no "rule", but based on observations on how people behave, e.g. * for a small patch like this that can be given a good review in 10 minutes or so, those who do not do so within the first 3 days will probably not do so. * once a reasonably thorough review is given, those who haven't responded to the patch and do not have much else to say are unlikely to respond. * on the other hand, after such a review is given, those who do not agree with the review tend to respond rather quickly, to get their voice in before it becomes too late. I would say it would be good to start working on it right away and use a couple of days reviewing it yourself before posting it. ``` ## Naomi Ibe, 2023-10-11 05:50 Subject: Re: [PATCH 1/1] [OUTREACHY] Fixed add.c file to conform to guidelines when using die() listed in issue #635 Message-ID: URL: https://gitlist.dev/e/CACS%3DG2wXkz9OyR5e0ADzWVy3ejibF-Js%3DsdJYJDRMh8qskO-9A%40mail.gmail.com In-Reply-To: ``` Okay then, I'll work with these guidelines in mind. Thank you very much On Tue, Oct 10, 2023 at 11:24 PM Junio C Hamano wrote: > > Naomi Ibe writes: > > > Thank you very much! I'd definitely make those changes on my next patch. > > [administrivia] do not top post. > > > Should I begin work on version 2 or should I still wait for additional > > input on the version 1? > > There is no "rule", but based on observations on how people behave, > e.g. > > * for a small patch like this that can be given a good review in 10 > minutes or so, those who do not do so within the first 3 days > will probably not do so. > > * once a reasonably thorough review is given, those who haven't > responded to the patch and do not have much else to say are > unlikely to respond. > > * on the other hand, after such a review is given, those who do not > agree with the review tend to respond rather quickly, to get > their voice in before it becomes too late. > > I would say it would be good to start working on it right away and > use a couple of days reviewing it yourself before posting it. ``` ## Dragan Simic, 2023-10-11 06:17 Subject: Re: [PATCH 1/1] [OUTREACHY] Fixed add.c file to conform to guidelines when using die() listed in issue #635 Message-ID: <89c51050c7570439f763f8fc8195aada@manjaro.org> URL: https://gitlist.dev/e/89c51050c7570439f763f8fc8195aada%40manjaro.org In-Reply-To: ``` On 2023-10-11 07:50, Naomi Ibe wrote: > Okay then, I'll work with these guidelines in mind. Thank you very much If I may notice, avoiding top posting has already been suggested twice, IIRC. I think that various webmails have made top posting popular, and actually forced people a bit to do that through the way their user interfaces were laid out, which made inline replies kind of a dying art, which is really sad to see. It is so much more useful when replies are posted inline, i.e. right below the contents they refer to. > On Tue, Oct 10, 2023 at 11:24 PM Junio C Hamano > wrote: >> >> Naomi Ibe writes: >> >> > Thank you very much! I'd definitely make those changes on my next patch. >> >> [administrivia] do not top post. >> >> > Should I begin work on version 2 or should I still wait for additional >> > input on the version 1? >> >> There is no "rule", but based on observations on how people behave, >> e.g. >> >> * for a small patch like this that can be given a good review in 10 >> minutes or so, those who do not do so within the first 3 days >> will probably not do so. >> >> * once a reasonably thorough review is given, those who haven't >> responded to the patch and do not have much else to say are >> unlikely to respond. >> >> * on the other hand, after such a review is given, those who do not >> agree with the review tend to respond rather quickly, to get >> their voice in before it becomes too late. >> >> I would say it would be good to start working on it right away and >> use a couple of days reviewing it yourself before posting it. ```