-
-
Notifications
You must be signed in to change notification settings - Fork 1.5k
Fix sync branch issue message and update contributing docs #15595
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: 2.5
Are you sure you want to change the base?
Conversation
| * Only push to release branches translation updates, use a PR for all the rest. | ||
| * Use pull requests to merge release branches. To ensure no duplication with the auto sync branch CI, it is recommended to push to the upstream repository (mixxxdj) with the branch name convention following `sync-branch-<SourceBranch>-to-<TargetBranch>` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The typo fix direct push shall be still allowed.
Do we really need to create manually upstream branches? For my understanding the CI skips the PR creation in case of conflicts.
| * Only push to release branches translation updates, use a PR for all the rest. | |
| * Use pull requests to merge release branches. To ensure no duplication with the auto sync branch CI, it is recommended to push to the upstream repository (mixxxdj) with the branch name convention following `sync-branch-<SourceBranch>-to-<TargetBranch>` | |
| * Only push directly to an upstream repository (mixxxdj) for trivial, uncontroversial changes like fixing a typo. | |
| * For merging release branches, use either the automatically created PRs or create manual PRs after conflict resolution. These PRs can be merged by the author after the CI is green. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The typo fix direct push shall be still allowed.
We use codespell to prevent those from ever being merged in so IMO, this is a rare enough case not to justify bypassing the PR process. I believe that most of the recent usecase where mostly about copy change, which should always goes via PR IMO.
For my understanding the CI skips the PR creation in case of conflicts.
No, the CI will only skip the creating if there is already an existing branch, thus why we need to use the same naming convention to prevent duplicate. The risk being, if the conflict somehow becomes solvable by a three way merge, the CI will create that PR.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I like to keep the first sentences as it is. This is unrelated to sync PRs, our current discussion.
Here another try:
| * Only push to release branches translation updates, use a PR for all the rest. | |
| * Use pull requests to merge release branches. To ensure no duplication with the auto sync branch CI, it is recommended to push to the upstream repository (mixxxdj) with the branch name convention following `sync-branch-<SourceBranch>-to-<TargetBranch>` | |
| * Only push directly to an upstream repository (mixxxdj) for trivial, uncontroversial changes like fixing a typo. | |
| * To merge release branches, either use the automatically created PR or, in case of conflicts, transfer the conflict resolution to the upstream repository (mixxxdj) with the branch name `sync-branch-<SourceBranch>-to-<TargetBranch>` and create a PR from it. |
This fixes the spacing problem spotted in the conflict detection issues. This also updates our contributing guideline to clarify the needs for PR for version sync.