On Tue, Oct 29, 2019 at 1:42 PM Emily Shaffer <emilyshaffer@xxxxxxxxxx> wrote:
>
> On Mon, Oct 28, 2019 at 11:42:29PM +0000, george espinoza via GitGitGadget wrote:
> > From: george espinoza <gespinoz2019@xxxxxxxxx>
> >
> > Teach this command which currently handles its own argv to use
> > parse-options instead because parse-options helps make sure we handle
> > user input like -h in a standardized way across the project.
> > Also deleted the NO_PARSEOPT flag from git.c to coincide with
> > the conversion of the structure in this command since merge-ours
> > now uses parse-options and needed to update git.c accordingly.
>
> Hmm, I could still wish for some rephrasing on this commit message. Now
> that you took my example suggestions and pasted them directly into your
> previous sentences, it doesn't flow very nicely. The point comes across
> but it's a little redundant (for example, "also <verb> from git.c ....
> and needed to update git.c" is redundant).
>
> When significant assistance and advice is received it's often good form
> to include a "Helped-by:" line which looks similar to the signoff line,
> to provide credit. Maybe you will consider it as myself and dscho spent
> quite some time helping you in the GitGitGadget PR as well as via
> IRC/mail? :)
>
> Otherwise, the code looks OK to me.
>
> - Emily
>
> >
> > Signed-off-by: george espinoza <gespinoz2019@xxxxxxxxx>
> > ---
> > builtin/merge-ours.c | 14 ++++++++++----
> > git.c | 2 +-
> > 2 files changed, 11 insertions(+), 5 deletions(-)
> >
> > diff --git a/builtin/merge-ours.c b/builtin/merge-ours.c
> > index 4594507420..fb3674a384 100644
> > --- a/builtin/merge-ours.c
> > +++ b/builtin/merge-ours.c
> > @@ -11,14 +11,20 @@
> > #include "git-compat-util.h"
> > #include "builtin.h"
> > #include "diff.h"
> > +#include "parse-options.h"
> >
> > -static const char builtin_merge_ours_usage[] =
> > - "git merge-ours <base>... -- HEAD <remote>...";
> > +static const char * const merge_ours_usage[] = {
> > + N_("git merge-ours [<base>...] -- <head> <merge-head>..."),
> > + NULL,
> > +};
> >
> > int cmd_merge_ours(int argc, const char **argv, const char *prefix)
> > {
> > - if (argc == 2 && !strcmp(argv[1], "-h"))
> > - usage(builtin_merge_ours_usage);
> > + struct option options[] = {
> > + OPT_END()
> > + };
> > +
> > + argc = parse_options(argc, argv, prefix, options, merge_ours_usage, 0);
> >
> > /*
> > * The contents of the current index becomes the tree we
> > diff --git a/git.c b/git.c
> > index ce6ab0ece2..6aee0e9477 100644
> > --- a/git.c
> > +++ b/git.c
> > @@ -527,7 +527,7 @@ static struct cmd_struct commands[] = {
> > { "merge-base", cmd_merge_base, RUN_SETUP },
> > { "merge-file", cmd_merge_file, RUN_SETUP_GENTLY },
> > { "merge-index", cmd_merge_index, RUN_SETUP | NO_PARSEOPT },
> > - { "merge-ours", cmd_merge_ours, RUN_SETUP | NO_PARSEOPT },
> > + { "merge-ours", cmd_merge_ours, RUN_SETUP },
> > { "merge-recursive", cmd_merge_recursive, RUN_SETUP | NEED_WORK_TREE | NO_PARSEOPT },
> > { "merge-recursive-ours", cmd_merge_recursive, RUN_SETUP | NEED_WORK_TREE | NO_PARSEOPT },
> > { "merge-recursive-theirs", cmd_merge_recursive, RUN_SETUP | NEED_WORK_TREE | NO_PARSEOPT },
> > --
> > gitgitgadget
Ah, my sincerest apologies. I should have been more thorough in
evaluating the importance of a clean original commit message before
submitting it. I will certainly keep all of that in mind for this
project and any future projects and contributions. I will edit it
accordingly to your advice and include credit for the substantial and
significant assistance you and dscho provided. :) ty.
-George
