Skip to content

docs: clarify pushing to branch etiquette #1436

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.

Sign up for GitHub

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

Open
majestic-owl448 wants to merge 1 commit into
base: main
Choose a base branch
from
+1 −1
Open

docs: clarify pushing to branch etiquette #1436

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/content/docs/basic-git-workflow.mdx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -226,7 +226,7 @@ Follow these steps:
</Steps>

:::caution
Do not repeatedly merge the `main` branch into your feature branch. Every merge commit triggers another round of CI runs that we have to pay for, and it clutters your pull request history. When you need to update your branch with the latest changes from `upstream/main`, [rebase](https://git-scm.com/book/en/v2/Git-Branching-Rebasing) instead. Also make sure your existing commits are squashed into a small set of meaningful commits before opening (or updating) your pull request.
Do not repeatedly merge the `main` branch into your feature branch. Every time something is pushed to the branch it triggers another round of CI runs that we have to pay for, and it clutters your pull request history. You don't usually need to update your branch with the latest changes from `upstream/main`, but if you do it, to minimize the CI runs please push try to this update with other commits that are updating your PR.please bundle the push with other When you need to update your branch with the latest changes from `upstream/main`, [rebase](https://git-scm.com/book/en/v2/Git-Branching-Rebasing) instead. Also, please do not rewrite the history of the commits already reviewed, that makes it harder on reviewers to distinguish new changes from old ones.

@raisedadead raisedadead Jul 16, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

The language is confusing and incoherent. As a contributor I am not going to bother reading all this let alone understanding it.

If we are needing to amend a caution to be off this length, then our original instructions outside the caution have a gap that we need to address.

Reword, make it concise to fit in one-two lines OR address the underlying instructions.

For example, ask:

  • What parts of the process is actually annoying to reviewers?
  • Why do we need to preserve previous commits at all (personally I think squashing and rebasing is fine)
  • If a follow up changes (seemingly after a review) amended the history, and the reviewer loses context, that's a deeper issue. Are the reviewer then only looking at the delta, and losing track of the whole PR patch – which will land when the PR is merged?, etc.
  • Can the instructions be made clear (outside the caution area). Add commits additively, we will squash them. etc.

@majestic-owl448 majestic-owl448 Jul 16, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

what is annoying - getting a notification that is just a merge commit, I guess

the issue with loosing track of the changes is mostly happening when PRs affect whole workshops (adding them or making changes), particularly because these can go on for weeks or months. Checking what was the last feedback left on the PR and being able to look at the "commits since last review" allow to see how the contributor is doing and if they need additional help. It is not strictly necessary, but it allows to see much faster if the PR is ready for an other full review or not.

Smaller PRs don't really need it, but adding too many caveats can be confusing. I will see about clarifying the page.

:::

## Proposing a Pull Request (PR)
Expand Down