Hey Siddharth, On Fri, Feb 3, 2017 at 11:58 PM, Siddharth Kannan <kannan.siddharth12@xxxxxxxxx> 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@xxxxxxxxx> > --- 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 > >