Hi Chris
On 23/09/2022 19:55, Stefan Xenos via GitGitGadget wrote:
From: Stefan Xenos <sxenos@xxxxxxxxxx>
Implement the git change update command, which
are sufficient for constructing change graphs.
For example, to create a new change (a stable name) that refers to HEAD:
git change update -c HEAD
To record a rebase or amend in the change graph:
git change update -c <new_commit> -r <old_commit>
To record a cherry-pick in the change graph:
git change update -c <new_commit> -o <original_commit>
While it is good to have this example it would be better to have some
documentation about how to use this command. It would be very helpful to
have the documentation added before the code so that reviewers have an
overview of the command when they come to review the code.
The commit message should also discuss why it is called "change" rather
than "evolve". For more details on commit messages for this project see
"Describe your changes well" in Documentation/SubmittingPatches. Having
some tests would make it clear how this command is intended to be used
as well as demonstrating that the implementation works.
Signed-off-by: Stefan Xenos <sxenos@xxxxxxxxxx>
Signed-off-by: Chris Poucet <poucet@xxxxxxxxxx>
---
+struct update_state {
+ int options;
+ const char* change;
+ const char* content;
+ struct string_list replace;
+ struct string_list origin;
+};
+
+static void init_update_state(struct update_state *state)
+{
+ memset(state, 0, sizeof(*state));
+ state->content = "HEAD";
+ string_list_init_nodup(&state->replace);
+ string_list_init_nodup(&state->origin);
+}
In general we prefer to use initializer macros over functions. So this
would become
#define UPDATE_STATE_INIT { \
.content = "HEAD", \
.replace = STRING_LIST_INIT_NODUP, \
.origin = STRING_LIST_INIT_NODUP \
}
and lower down we'd have
struct update_state state = UPDATE_STATE_INIT;
Likewise we prefer
struct foo = { 0 };
over
struct foo foo;
memset(&foo, 0, sizeof(foo));
+int cmd_change(int argc, const char **argv, const char *prefix)
+{
+ /* No options permitted before subcommand currently */
+ struct option options[] = {
+ OPT_END()
+ };
+ int result = 1;
+
+ argc = parse_options(argc, argv, prefix, options, builtin_change_usage,
+ PARSE_OPT_STOP_AT_NON_OPTION);
+
+ if (argc < 1)
+ usage_with_options(builtin_change_usage, options);
+ else if (!strcmp(argv[0], "update"))
+ result = change_update(argc, argv, prefix);
Since Stefan wrote this code the parse options api has been improved to
support sub commands so this should be updated to use that support.
Thanks again for picking up these patches, I'm excited to see an evolve
command for git.
Best Wishes
Phillip
