tier4 / AWSIM

Open source simulator for self-driving vehicles
https://tier4.github.io/AWSIM/
Other
505 stars 97 forks source link

Please require linear history for the main branch #272

Open xmfcx opened 7 months ago

xmfcx commented 7 months ago

https://docs.github.com/en/repositories/configuring-branches-and-merges-in-your-repository/managing-protected-branches/about-protected-branches#require-linear-history

Enforcing a linear commit history prevents collaborators from pushing merge commits to the branch. This means that any pull requests merged into the protected branch must use a squash merge or a rebase merge. A strictly linear commit history can help teams reverse changes more easily. For more information about merge methods, see "About pull request merges."

Before you can require a linear commit history, your repository must allow squash merging or rebase merging. For more information, see "Configuring pull request merges."

This will make sure the git history is linearly stacked neatly like we have in Autoware Universe: Screenshot from 2024-03-05 12-39-33

But AWSIM history looks like a tangled mess spaghetti: Screenshot from 2024-03-05 12-37-41

To enable this setting, browse to one of the following, whichever you are using:

And enable Require linear history rule.

@mackierx111

mitsudome-r commented 3 months ago

@mackierx111 @YoshinoriTsutake This topic was raised again by Fatih during Simulation WG. It seems like it's making hard for us to merge some features from upstream to our fork as well.

To help us out, could you answer the following questions?

  1. Are there any reasons why we are not using squash merge when merging PRs?

I think in the internal TIER IV slack, you mentioned about issues with loss of traceability, for example with git blame, but I don't think this would be an issue if the PRs are small enough. At least I went through the changelogs in the latest release, and it seems the PRs are kept in meaningful units so that I don't really see any reasons for keeping the original commits. We could still look back into the Pull Request thread if you want to know the details anyways.

  1. Is there a recommended way of cherry-picking a feature from upstream to the fork repo?

For example, if we just want to merge this regression test into a fork, we have to cherry-pick four commits just to merge this single feature. If it is only four commits, it is still manageable, but it starts to become more difficult if the original PR is made of number of commits. Merging the original branch is also difficult because it may contain some other commits from the main branch which we might not want to merge yet in our fork.

If it is not possible to "require" linear history, is it possible to use squash merge as a default (or recommended) way of merging the PR? We could still use merging with merge commits for the PRs that would like to keep the individual commits from the feature branch if there are reasons to do so, but I don't think that's the case for most of the PRs that we have for AWSIM.