Re: [PATCH 1/5] Add a new option 'merges' to revision.c
- From
Eric Sunshine <sunshine@sunshineco.com>
- Date
- Mar 22, 2015, 20:43 UTC
- Message-ID
- <CAPig+cRKc59804+yXtX8ZJagsJ4NVw-61Ru1kh1eBusGR1Ai_g@mail.gmail.com>
- In-Reply-To
- <1427048921-28677-1-git-send-email-koosha@posteo.de>
On Sun, Mar 22, 2015 at 2:28 PM, Koosha Khajehmoogahi <koosha@posteo.de> wrote:
> Subject: Add a new option 'merges' to revision.c
For the subject, mention the area you're touching, followed by a colon, followed by a short but meaningful summary of the change. Drop capitalization.
revision: add --merges={show|only|hide} option> revision.c: add a new option 'merges' with
No need to mention "revision.c" here since the patch itself shows this clearly.
Considering that there is already a --merges option, it is somewhat misleading and not terribly clear to say only that you're adding a new "option 'merges'". Better would be to spell out --merges= explicitly.
Show 8 quoted lines
> possible values of 'only', 'show' and 'hide'. > The option is used when showing the list of commits. > The value 'only' lists only merges. The value 'show' > is the default behavior which shows the commits as well > as merges and the value 'hide' makes it just list commit > items. > > Signed-off-by: Koosha Khajehmoogahi <koosha@posteo.de>
Considering that Junio actually wrote this patch[1], it would be more correct and considerate to attribute it to him. You can do so by ensuring that there is a "From:" header at the very top of the commit message before mailing out the patch:
From: Junio C Hamano <gitster@pobox.com>
The customary way to indicate that you wrote the commit message and made a few small adjustments to Junio's patch would be with a bracketed comment (starting with your initials) just before your sign-off. Something like this:
[kk: wrote commit message; added a couple missing
{min|max}_parents assignments][1]: http://article.gmane.org/gmane.comp.version-control.git/265597
Show 58 quoted lines
> ---
> diff --git a/revision.c b/revision.c
> index 66520c6..edb7bed 100644
> --- a/revision.c
> +++ b/revision.c
> @@ -1678,6 +1678,23 @@ static void add_message_grep(struct rev_info *revs, const char *pattern)
> add_grep(revs, pattern, GREP_PATTERN_BODY);
> }
>
> +int parse_merges_opt(struct rev_info *revs, const char *param)
> +{
> + if (!strcmp(param, "show")) {
> + revs->min_parents = 0;
> + revs->max_parents = -1;
> + } else if (!strcmp(param, "only")) {
> + revs->min_parents = 2;
> + revs->max_parents = -1;
> + } else if (!strcmp(param, "hide")) {
> + revs->min_parents = 0;
> + revs->max_parents = 1;
> + } else {
> + return -1;
> + }
> + return 0;
> +}
> +
> static int handle_revision_opt(struct rev_info *revs, int argc, const char **argv,
> int *unkc, const char **unkv)
> {
> @@ -1800,9 +1817,14 @@ static int handle_revision_opt(struct rev_info *revs, int argc, const char **arg
> revs->show_all = 1;
> } else if (!strcmp(arg, "--remove-empty")) {
> revs->remove_empty_trees = 1;
> + } else if (starts_with(arg, "--merges=")) {
> + if (parse_merges_opt(revs, arg + 9))
> + die("unknown option: %s", arg);
> } else if (!strcmp(arg, "--merges")) {
> + revs->max_parents = -1;
> revs->min_parents = 2;
> } else if (!strcmp(arg, "--no-merges")) {
> + revs->min_parents = 0;
> revs->max_parents = 1;
> } else if (starts_with(arg, "--min-parents=")) {
> revs->min_parents = atoi(arg+14);
> diff --git a/revision.h b/revision.h
> index 0ea8b4e..f9df58c 100644
> --- a/revision.h
> +++ b/revision.h
> @@ -240,6 +240,7 @@ extern int setup_revisions(int argc, const char **argv, struct rev_info *revs,
> extern void parse_revision_opt(struct rev_info *revs, struct parse_opt_ctx_t *ctx,
> const struct option *options,
> const char * const usagestr[]);
> +extern int parse_merges_opt(struct rev_info *, const char *);
> #define REVARG_CANNOT_BE_FILENAME 01
> #define REVARG_COMMITTISH 02
> extern int handle_revision_arg(const char *arg, struct rev_info *revs,
> --
> 2.3.3.263.g095251d.dirty