Re: [PATCH 2/2] merge: warn --no-commit merge when no new commit is created

From: Stefan Beller <sbeller@google.com>
Date: Apr 26, 2016, 21:53 UTC
Message-ID: <CAGZ79ka-HbB=oCidTtKSa32R9kqd2_c-cG8h+ttAVy=a8UfgFQ@mail.gmail.com>
In-Reply-To: <xmqqd1pcdpb8.fsf_-_@gitster.mtv.corp.google.com>

On Tue, Apr 26, 2016 at 2:37 PM, Junio C Hamano <gitster@pobox.com> wrote:

Show 8 quoted lines

> +static void no_commit_impossible(const char *message)
> +{
> +       if (!option_commit) {
> +               warning("%s\n%s", _(message),
> +                       _("--no-commit is impossible"));
> +               warning(_("In future versions of Git, this will become an error."));
> +       }
> +}

During discussion of the parallel process framework (sb/submodule-parallel-fetch~3), you seemed very inclined on not having major decisions made deep inside the helper function, but rather at the main function to easier see the program flow IIRC.

This looks very similar to me as we'll have the no_commit_impossible function which is a helper of cmd_merge. Following your advice there, I would have expected to have

    static void no_commit_impossible(const char *message)
    {
        warning("%s\n%s", _(message), _("--no-commit is impossible"));
        warning(_("In future versions of Git, this will become an error."));
    }

and later

    if (!option_commit)
        no_commit_impossible(_("Already up-to-date"));

Show 10 quoted lines

> +
>  int cmd_merge(int argc, const char **argv, const char *prefix)
>  {
>         unsigned char result_tree[20];
> @@ -1403,6 +1412,7 @@ int cmd_merge(int argc, const char **argv, const char *prefix)
>                  * If head can reach all the merge then we are up to date.
>                  * but first the most common case of merging one remote.
>                  */
> +               no_commit_impossible(_("Already up-to-date"));
>                 finish_up_to_date("Already up-to-date.");

Coming back to this patch, in case of -v given, we'll see ("Already up-to-date") twice?

If --quiet is given, do we want to suppress output in no_commit_impossible?