Skip to content

Conversation

@SandrineP
Copy link
Collaborator

Add merge commit

@SandrineP SandrineP marked this pull request as ready for review October 31, 2025 10:50
else
{
msg.append("commit ");
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These two "if/else" sections can be merged into a single one. Or you could use the ternary operator to initilaze the message in a single line:

std::string msg = merge_ref ? "Merge branch" : "Merge commit'";

}

void create_merge_commit(repository_wrapper& repo, const index_wrapper& index, std::vector<std::string> m_branches_to_merge,
const annotated_commit_list_wrapper& commits_to_merge, size_t num_commits_to_merge)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

create_merge_commit could be a method of the merge_subcommand class; this way it would have access to the m_changes_to_merge member, and this parameter can be removed. If we prefer to keep it as a free function, then the parameter should be renamed in branches_to_merge (the m_ prefix should be reserved for class members).

Copy link
Member

@ianthomas23 ianthomas23 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @SandrineP

@ianthomas23 ianthomas23 merged commit 437e2b7 into QuantStack:main Oct 31, 2025
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants