Re: [PATCH] git-parse-remote.sh: Remove op_prep argument

[Pranit Bauva <pranit.bauva@gmail.com>]

Hey Siddharth,

On Fri, Feb 3, 2017 at 11:58 PM, Siddharth Kannan
<kannan.siddharth12@gmail.com> wrote:
> - Remove the third argument of error_on_missing_default_upstream that is no
>   longer required
> - FIXME to remove this argument was added in commit 045fac5845

This is not exactly correct. Well, this is the commit you get on
git-blame but it isn't really the one to look for. The "real" commit
when the variable was introduced was 15a147e61898 and was used for
writing out the error message. The commit 045fac5845 changed the error
message and the variable was not used then so it got redundant. So if
possible you could document all this information in the commit message
somehow, then it would be really great! :)

> - Run "grep" on the rest of the codebase to find and remove occurences of

/s/occurences/occurrences/g     (spelling mistake ;))

>   the third argument and fix the function calls appropriately
>
> Signed-off-by: Siddharth Kannan <kannan.siddharth12@gmail.com>
> ---

So if you want a better commit message then you could probably use this,
"parse-remote: remove reference to unused op_prep

This argument was introduced in the commit 15a147e618 to help in
writing out the error message but then in commit 045fac5845, the
reference to op_prep got removed. Thus the argument is no longer
useful and is removed.
"

> The contrib/examples/git-pull.sh file also has a variable op_prep which is used
> in one of the messages shown the user. Should I remove this variable as well?

Not really. We have kept the file git-pull.sh just as an example of
how git-pull was initially implemented. So previously git-pull was a
shell script which was then ported to C code. After that conversion,
the shell script was just put as it is in contrib/examples/ as a use
case of how git-pull should be implemented. I don't think there is any
need to modify it, but there isn't really a very strong reason to not
modify it (except that we don't usually write out the new changes to
it).

>  contrib/examples/git-pull.sh | 2 +-
>  git-parse-remote.sh          | 3 +--
>  git-rebase.sh                | 2 +-
>  3 files changed, 3 insertions(+), 4 deletions(-)
>
> diff --git a/contrib/examples/git-pull.sh b/contrib/examples/git-pull.sh
> index 6b3a03f..1d51dc3 100755
> --- a/contrib/examples/git-pull.sh
> +++ b/contrib/examples/git-pull.sh
> @@ -267,7 +267,7 @@ error_on_no_merge_candidates () {
>                 echo "for your current branch, you must specify a branch on the command line."
>         elif [ -z "$curr_branch" -o -z "$upstream" ]; then
>                 . git-parse-remote
> -               error_on_missing_default_upstream "pull" $op_type $op_prep \
> +               error_on_missing_default_upstream "pull" $op_type \
>                         "git pull <remote> <branch>"
>         else
>                 echo "Your configuration specifies to $op_type $op_prep the ref '${upstream#refs/heads/}'"
> diff --git a/git-parse-remote.sh b/git-parse-remote.sh
> index d3c3998..9698a05 100644
> --- a/git-parse-remote.sh
> +++ b/git-parse-remote.sh
> @@ -56,8 +56,7 @@ get_remote_merge_branch () {
>  error_on_missing_default_upstream () {
>         cmd="$1"
>         op_type="$2"
> -       op_prep="$3" # FIXME: op_prep is no longer used
> -       example="$4"
> +       example="$3"
>         branch_name=$(git symbolic-ref -q HEAD)
>         display_branch_name="${branch_name#refs/heads/}"
>         # If there's only one remote, use that in the suggestion
> diff --git a/git-rebase.sh b/git-rebase.sh
> index 04f6e44..b89f960 100755
> --- a/git-rebase.sh
> +++ b/git-rebase.sh
> @@ -448,7 +448,7 @@ then
>                 then
>                         . git-parse-remote
>                         error_on_missing_default_upstream "rebase" "rebase" \
> -                               "against" "git rebase $(gettext '<branch>')"
> +                               "git rebase $(gettext '<branch>')"
>                 fi
>
>                 test "$fork_point" = auto && fork_point=t
> --
> 2.1.4
>
>