Skip to contentSkip to conversation

sequencer: remove unnecessary variable setting

PATCH In review Elijah Newren via GitGitGadget 3 participants 3 messages Aug 12 – Aug 13

lore
Share & tools
lore
#0 lore raw

PATCH 1/1 sequencer: remove unnecessary variable setting

From: Elijah Newren <[email protected]>

revs.pretty_given is only ever read in builtin/log.c, and nothing from builtin/log.c is ever called from sequencer.c. So setting this variable cannot do anything.

This was introduced in commit 62db524779 ("rebase -i: generate the script via rebase--helper", 2017-07-14), which used git rev-list even though its commit message describes the logic as having been based on git log. Because of this, I am guessing this line was copied or ported from part of builtin/log.c without recognizing that this line was not doing anything and could be removed.

It's certainly not doing anything now, though, so remove it.

---
sequencer: remove unnecessary variable setting

Random thing I noticed a few years ago, I believe while investigating our tangled web of revision fields and parsing. Either way, it's still valid and I'm finally sending it upstream.

Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1922%2Fnewren%2Fsequencer-remove-unnecessary-setting-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1922/newren/sequencer-remove-unnecessary-setting-v1
Pull-Request: https://github.com/gitgitgadget/git/pull/1922

 sequencer.c | 1 -
 1 file changed, 1 deletion(-)
sequencer.c +0 -1
index 83c3849205..a0abcc69ce 100644
@@ -6277,7 +6277,6 @@ int sequencer_make_script(struct repository *r, struct strbuf *out,
 	revs.sort_order = REV_SORT_IN_GRAPH_ORDER;
 	revs.topo_order = 1;
 
-	revs.pretty_given = 1;
 	repo_config_get_string(the_repository, "rebase.instructionFormat", &format);
 	if (!format || !*format) {
 		free(format);

base-commit: 2c78326f810173a4f3aefd8021f1e07575412481

Signature · 1 line
gitgitgadget
#1 lore raw
Quoted 5 lines · Elijah Newren via GitGitGadget

From: Elijah Newren <[email protected]>

revs.pretty_given is only ever read in builtin/log.c, and nothing from builtin/log.c is ever called from sequencer.c. So setting this variable cannot do anything.

Thanks. I'll mark the topic for 'next'.

Quoted 38 lines

This was introduced in commit 62db524779 ("rebase -i: generate the script via rebase--helper", 2017-07-14), which used git rev-list even though its commit message describes the logic as having been based on git log. Because of this, I am guessing this line was copied or ported from part of builtin/log.c without recognizing that this line was not doing anything and could be removed.

It's certainly not doing anything now, though, so remove it.

---
sequencer: remove unnecessary variable setting

Random thing I noticed a few years ago, I believe while investigating our tangled web of revision fields and parsing. Either way, it's still valid and I'm finally sending it upstream.

Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1922%2Fnewren%2Fsequencer-remove-unnecessary-setting-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1922/newren/sequencer-remove-unnecessary-setting-v1
Pull-Request: https://github.com/gitgitgadget/git/pull/1922

 sequencer.c | 1 -
 1 file changed, 1 deletion(-)
sequencer.c +0 -1
index 83c3849205..a0abcc69ce 100644
@@ -6277,7 +6277,6 @@ int sequencer_make_script(struct repository *r, struct strbuf *out,
 	revs.sort_order = REV_SORT_IN_GRAPH_ORDER;
 	revs.topo_order = 1;
 
-	revs.pretty_given = 1;
 	repo_config_get_string(the_repository, "rebase.instructionFormat", &format);
 	if (!format || !*format) {
 		free(format);

base-commit: 2c78326f810173a4f3aefd8021f1e07575412481

#2 lore raw
In reply to Elijah Newren via GitGitGadget

sequencer.c int sequencer_make_script(struct repository *r, struct strbuf *out, · line 6282

Quoted 12 lines · Elijah Newren via GitGitGadget
sequencer.c +0 -1
index 83c3849205..a0abcc69ce 100644
@@ -6277,7 +6277,6 @@ int sequencer_make_script(struct repository *r, struct strbuf *out,
 	revs.sort_order = REV_SORT_IN_GRAPH_ORDER;
 	revs.topo_order = 1;
 
-	revs.pretty_given = 1;
 	repo_config_get_string(the_repository, "rebase.instructionFormat", &format);
 	if (!format || !*format) {
 		free(format);

Makes sense. The only reference to this field is indeed in "builtin/log.c", and as we don't use the sequencer there shouldn't be any kind of interaction between those two subsystems here.

Thanks!

Patrick