| Message ID | 20201214202647.3340193-4-gitster@pobox.com (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | b044db9172f020768e85835f87bfc5564e310714 |
| Headers | show |
| Series | making pull advice not to trigger when unneeded | expand |
Junio C Hamano wrote: > It is easy enough to do, Yes. > gives a more descriptive name to the variable, I disagree. > and there is no reason to make the code deliberately worse by ignoring > improvement offered on the list. I doubt any person contributing to the mailing is making the code deliberately worse. And I certainly did not ignore any improvement on the list. I responded to the suggestion, I just disagreed it's actually an improvement. In my opinion differences of opinion must be tolerated. And I don't think any of that belongs in the commit message. "Gives a more descriptive name to the variable" (in your opinion) should be enough. Anyway, I like the fact that your opinion and my opinion are clearly demarcated in two different commits. Cheers.
Felipe Contreras <felipe.contreras@gmail.com> writes: > Junio C Hamano wrote: >> It is easy enough to do, > > Yes. > >> gives a more descriptive name to the variable, > > I disagree. > >> and there is no reason to make the code deliberately worse by ignoring >> improvement offered on the list. > > I doubt any person contributing to the mailing is making the code > deliberately worse. Oh, I doubt that you do so with what you send out. I am saying that you do so by not taking improvements. It wastes reviewers' time, raises noise ratio in the list traffic, and demotivates readers.
Junio C Hamano wrote: > Felipe Contreras <felipe.contreras@gmail.com> writes: > > Junio C Hamano wrote: > >> It is easy enough to do, > > > > Yes. > > > >> gives a more descriptive name to the variable, > > > > I disagree. > > > >> and there is no reason to make the code deliberately worse by ignoring > >> improvement offered on the list. > > > > I doubt any person contributing to the mailing is making the code > > deliberately worse. > > Oh, I doubt that you do so with what you send out. I am saying that > you do so by not taking improvements. I do take improvements, when I agree they are improvements. In fact I did take virtually all of Elijah Newren's improvements. > It wastes reviewers' time, raises noise ratio in the list traffic, and > demotivates readers. Are you saying I must always agree with you, or I waste your time? In my view no one is infallible, just because X person says Y is an improvement that doesn't necessarily mean it actually is. I thought this was a collaborative process where you are supposed to listen to my feedback to your suggestions too. But I guess I shall take your "suggestions" as *orders*.
diff --git a/builtin/pull.c b/builtin/pull.c index ff8e3ce137..2976b8e5cb 100644 --- a/builtin/pull.c +++ b/builtin/pull.c @@ -27,8 +27,6 @@ #include "commit-reach.h" #include "sequencer.h" -static int default_mode; - /** * Parses the value of --rebase. If value is a false value, returns * REBASE_FALSE. If value is a true value, returns REBASE_TRUE. If value is @@ -326,7 +324,7 @@ static const char *config_get_ff(void) * looks for the value of "pull.rebase". If both configuration keys do not * exist, returns REBASE_FALSE. */ -static enum rebase_type config_get_rebase(void) +static enum rebase_type config_get_rebase(int *rebase_unspecified) { struct branch *curr_branch = branch_get("HEAD"); const char *value; @@ -346,7 +344,7 @@ static enum rebase_type config_get_rebase(void) if (!git_config_get_value("pull.rebase", &value)) return parse_config_rebase("pull.rebase", value, 1); - default_mode = 1; + *rebase_unspecified = 1; return REBASE_FALSE; } @@ -934,6 +932,7 @@ int cmd_pull(int argc, const char **argv, const char *prefix) struct object_id orig_head, curr_head; struct object_id rebase_fork_point; int autostash; + int rebase_unspecified = 0; if (!getenv("GIT_REFLOG_ACTION")) set_reflog_message(argc, argv); @@ -955,7 +954,7 @@ int cmd_pull(int argc, const char **argv, const char *prefix) opt_ff = xstrdup_or_null(config_get_ff()); if (opt_rebase < 0) - opt_rebase = config_get_rebase(); + opt_rebase = config_get_rebase(&rebase_unspecified); if (read_cache_unmerged()) die_resolve_conflict("pull"); @@ -1029,7 +1028,7 @@ int cmd_pull(int argc, const char **argv, const char *prefix) if (opt_rebase && merge_heads.nr > 1) die(_("Cannot rebase onto multiple branches.")); - if (default_mode && opt_verbosity >= 0 && !opt_ff) { + if (rebase_unspecified && opt_verbosity >= 0 && !opt_ff) { advise(_("Pulling without specifying how to reconcile divergent branches is\n" "discouraged. You can squelch this message by running one of the following\n" "commands sometime before your next pull:\n"
It is easy enough to do, gives a more descriptive name to the variable, and there is no reason to make the code deliberately worse by ignoring improvement offered on the list. Signed-off-by: Junio C Hamano <gitster@pobox.com> --- builtin/pull.c | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-)