| Message ID | 20181207235425.128568-4-sbeller@google.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [1/4] submodule update: add regression test with old style setups | expand |
Stefan Beller <sbeller@google.com> writes: > Shortly after f178c13fda (Revert "Merge branch > 'sb/submodule-core-worktree'", 2018-09-07), we had another series > that implemented partially the same, ensuring that core.worktree was > set in a checked out submodule, namely 74d4731da1 (submodule--helper: > replace connect-gitdir-workingtree by ensure-core-worktree, 2018-08-13) > > As the series 4d6d6ef1fc (Merge branch 'sb/submodule-update-in-c', > 2018-09-17) has different goals than the reverted series 7e25437d35 > (Merge branch 'sb/submodule-core-worktree', 2018-07-18), I'd wanted to > replay the series on top of it to reach the goal of having `core.worktree` > correctly set when the submodules worktree is present, and unset when the > worktree is not present. > > The replay resulted in a strange merge conflict highlighting that > the BUG message was not changed in 74d4731da1 (submodule--helper: > replace connect-gitdir-workingtree by ensure-core-worktree, 2018-08-13). > > Fix the error message. > > Signed-off-by: Stefan Beller <sbeller@google.com> > --- Unlike the step 2/4 I commented on, this does explain what this wants to do and why, at least when looked from sideways. Is the above saying the same as the following two-liner? An ealier mistake while rebasing to produce 74d4731da1 failed to update this BUG message. Fix this. > builtin/submodule--helper.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c > index d38113a31a..31ac30cf2f 100644 > --- a/builtin/submodule--helper.c > +++ b/builtin/submodule--helper.c > @@ -2045,7 +2045,7 @@ static int ensure_core_worktree(int argc, const char **argv, const char *prefix) > struct repository subrepo; > > if (argc != 2) > - BUG("submodule--helper connect-gitdir-workingtree <name> <path>"); > + BUG("submodule--helper ensure-core-worktree <path>"); > > path = argv[1];
> Unlike the step 2/4 I commented on, this does explain what this > wants to do and why, at least when looked from sideways. Is the > above saying the same as the following two-liner? > > An ealier mistake while rebasing to produce 74d4731da1 > failed to update this BUG message. Fix this. I am not sure if it was rebasing, which was executed mistakenly. So maybe just saying "74d4731da1 contains a faulty BUG message. Fix it." would do. The intent of the longer message was to shed light in how I found the BUG (ie. I did not see the BUG message, which would ask me to actually fix a bug, but found it via code inspection), which I thought was valuable information, too.
Stefan Beller <sbeller@google.com> writes: >> Unlike the step 2/4 I commented on, this does explain what this >> wants to do and why, at least when looked from sideways. Is the >> above saying the same as the following two-liner? >> >> An ealier mistake while rebasing to produce 74d4731da1 >> failed to update this BUG message. Fix this. > > I am not sure if it was rebasing, which was executed mistakenly. > So maybe just saying "74d4731da1 contains a faulty BUG > message. Fix it." would do. > > The intent of the longer message was to shed light in how I found > the BUG (ie. I did not see the BUG message, which would ask me > to actually fix a bug, but found it via code inspection), which I > thought was valuable information, too. I guess that it could be stated in a way to make it valuable, but in the presented text, I somehow found it was making the more important part of the description (i.e. "this patch fixes a mistake made by 74d4731da1") buried and harder to grok. Thanks.
diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c index d38113a31a..31ac30cf2f 100644 --- a/builtin/submodule--helper.c +++ b/builtin/submodule--helper.c @@ -2045,7 +2045,7 @@ static int ensure_core_worktree(int argc, const char **argv, const char *prefix) struct repository subrepo; if (argc != 2) - BUG("submodule--helper connect-gitdir-workingtree <name> <path>"); + BUG("submodule--helper ensure-core-worktree <path>"); path = argv[1];
Shortly after f178c13fda (Revert "Merge branch 'sb/submodule-core-worktree'", 2018-09-07), we had another series that implemented partially the same, ensuring that core.worktree was set in a checked out submodule, namely 74d4731da1 (submodule--helper: replace connect-gitdir-workingtree by ensure-core-worktree, 2018-08-13) As the series 4d6d6ef1fc (Merge branch 'sb/submodule-update-in-c', 2018-09-17) has different goals than the reverted series 7e25437d35 (Merge branch 'sb/submodule-core-worktree', 2018-07-18), I'd wanted to replay the series on top of it to reach the goal of having `core.worktree` correctly set when the submodules worktree is present, and unset when the worktree is not present. The replay resulted in a strange merge conflict highlighting that the BUG message was not changed in 74d4731da1 (submodule--helper: replace connect-gitdir-workingtree by ensure-core-worktree, 2018-08-13). Fix the error message. Signed-off-by: Stefan Beller <sbeller@google.com> --- builtin/submodule--helper.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)