| Message ID | pull.1190.v2.git.1648752601.gitgitgadget@gmail.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | branch --recurse-submodules: Bug fixes and clean ups | expand |
"Glen Choo via GitGitGadget" <gitgitgadget@gmail.com> writes: > Thanks for the feedback, all. This version incorporates most of the > suggestions (which were pretty small anyway). > > == Patch organization > > Patches 1-2 are bugfixes, 3-4 are clean ups. > > Patch 1 fixes a bug where "git branch --recurse-submodules" would not > propagate the "--track" option if its value was "--no-track" or > "--track=inherit". > > Patch 2 fixes a bug where "git branch --recurse-submodules" would give > advice before telling the user what the problem is (instead of the other way > around). > > Patch 3 fixes some old inconsistencies when "git branch --set-upstream-to" > gives advice and when it doesn't. > > Patch 4 replaces exit(-1) with exit(1). > > == Changes > > Since v1: > > * Patch 1: reword the --track comments to be prescriptive > * Patch 3: remove a now-unnecessary die(). I didn't include a suggestion to > inline the advice string to save reviewers the trouble of proofreading > (and the format string has no placeholders anyway, so I don't think we'd > get much benefit out of typechecking). We can inline it in another > series. Thanks, but sorry that I've already merged the previous round. Let me turn them into incrementals.
Junio C Hamano <gitster@pobox.com> writes: > "Glen Choo via GitGitGadget" <gitgitgadget@gmail.com> writes: > >> Thanks for the feedback, all. This version incorporates most of the >> suggestions (which were pretty small anyway). >> >> == Patch organization >> >> Patches 1-2 are bugfixes, 3-4 are clean ups. >> >> Patch 1 fixes a bug where "git branch --recurse-submodules" would not >> propagate the "--track" option if its value was "--no-track" or >> "--track=inherit". >> >> Patch 2 fixes a bug where "git branch --recurse-submodules" would give >> advice before telling the user what the problem is (instead of the other way >> around). >> >> Patch 3 fixes some old inconsistencies when "git branch --set-upstream-to" >> gives advice and when it doesn't. >> >> Patch 4 replaces exit(-1) with exit(1). >> >> == Changes >> >> Since v1: >> >> * Patch 1: reword the --track comments to be prescriptive >> * Patch 3: remove a now-unnecessary die(). I didn't include a suggestion to >> inline the advice string to save reviewers the trouble of proofreading >> (and the format string has no placeholders anyway, so I don't think we'd >> get much benefit out of typechecking). We can inline it in another >> series. > > Thanks, but sorry that I've already merged the previous round. Let > me turn them into incrementals. Ah whoops, sorry I didn't send them out sooner. Thanks for that :)
Glen Choo <chooglen@google.com> writes: >> Thanks, but sorry that I've already merged the previous round. Let >> me turn them into incrementals. > > Ah whoops, sorry I didn't send them out sooner. No, it was me this time. I have prepared 'next' long in advance so I could have redone it if I delayed pushing the result out, but in any case, we are working asynchronously and these things are bound to happen. > Thanks for that :) Thanks for working on the topic.
Thanks for the feedback, all. This version incorporates most of the suggestions (which were pretty small anyway). == Patch organization Patches 1-2 are bugfixes, 3-4 are clean ups. Patch 1 fixes a bug where "git branch --recurse-submodules" would not propagate the "--track" option if its value was "--no-track" or "--track=inherit". Patch 2 fixes a bug where "git branch --recurse-submodules" would give advice before telling the user what the problem is (instead of the other way around). Patch 3 fixes some old inconsistencies when "git branch --set-upstream-to" gives advice and when it doesn't. Patch 4 replaces exit(-1) with exit(1). == Changes Since v1: * Patch 1: reword the --track comments to be prescriptive * Patch 3: remove a now-unnecessary die(). I didn't include a suggestion to inline the advice string to save reviewers the trouble of proofreading (and the format string has no placeholders anyway, so I don't think we'd get much benefit out of typechecking). We can inline it in another series. Glen Choo (4): branch: support more tracking modes when recursing branch: give submodule updating advice before exit branch --set-upstream-to: be consistent when advising branch: remove negative exit code branch.c | 47 +++++++++++++++++++++++++++---------- builtin/submodule--helper.c | 7 +++--- t/t3207-branch-submodule.sh | 38 +++++++++++++++++++++++++++++- 3 files changed, 76 insertions(+), 16 deletions(-) base-commit: abf474a5dd901f28013c52155411a48fd4c09922 Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1190%2Fchooglen%2Fbranch%2Frecursive-fixes-v2 Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1190/chooglen/branch/recursive-fixes-v2 Pull-Request: https://github.com/gitgitgadget/git/pull/1190 Range-diff vs v1: 1: 0b682c173c8 ! 1: 5e96a09199f branch: support more tracking modes when recursing @@ branch.c: static int submodule_create_branch(struct repository *r, + strvec_push(&child.args, "--track=inherit"); + break; + case BRANCH_TRACK_UNSPECIFIED: -+ /* Default for "git checkout". No need to pass --track. */ ++ /* Default for "git checkout". Do not pass --track. */ + case BRANCH_TRACK_REMOTE: -+ /* Default for "git branch". No need to pass --track. */ ++ /* Default for "git branch". Do not pass --track. */ + break; + } 2: f21d283e5ad = 2: 74b839bfc4e branch: give submodule updating advice before exit 3: 8e6ea3478b3 ! 3: 64d9b8e0f44 branch --set-upstream-to: be consistent when advising @@ Commit message [1] https://lore.kernel.org/git/211210.86ee6ldwlc.gmgdl@evledraar.gmail.com + Helped-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Signed-off-by: Glen Choo <chooglen@google.com> ## branch.c ## @@ branch.c: static void dwim_branch_start(struct repository *r, const char *start_name, + real_ref = NULL; if (get_oid_mb(start_name, &oid)) { if (explicit_tracking) { - if (advice_enabled(ADVICE_SET_UPSTREAM_FAILURE)) { +- if (advice_enabled(ADVICE_SET_UPSTREAM_FAILURE)) { - error(_(upstream_missing), start_name); -+ int code = die_message(_(upstream_missing), -+ start_name); - advise(_(upstream_advice)); +- advise(_(upstream_advice)); - exit(1); -+ exit(code); - } - die(_(upstream_missing), start_name); +- } +- die(_(upstream_missing), start_name); ++ int code = die_message(_(upstream_missing), start_name); ++ advise_if_enabled(ADVICE_SET_UPSTREAM_FAILURE, ++ _(upstream_advice)); ++ exit(code); } + die(_("not a valid object name: '%s'"), start_name); + } 4: fb2b472d9ae = 4: 306858373cc branch: remove negative exit code