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

[Stefan Beller <sbeller@xxxxxxxxxx>]

On Tue, Apr 26, 2016 at 2:37 PM, Junio C Hamano <gitster@xxxxxxxxx> wrote:
> +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"));


> +
>  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?
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html