Clarify git workflow: push to PR branch, don't create separate branches#33963
Clarify git workflow: push to PR branch, don't create separate branches#33963
Conversation
There was a problem hiding this comment.
Pull request overview
Updates the repository’s Copilot CLI git workflow guidance to clarify how to contribute fixes to an existing PR, aiming to prevent creating “branch-off-a-PR-branch” situations that don’t update the original PR (and therefore don’t run CI for it).
Changes:
- Add a new Git Workflow rule instructing commits to be made directly on the existing PR branch when fixing CI / amending a PR.
- Renumber existing git workflow rules accordingly.
- Add an example workflow using
gh pr checkoutfollowed by commit and push.
.github/copilot-instructions.md
Outdated
|
|
||
| **When fixing CI or contributing to an existing PR:** | ||
| ```bash | ||
| # Checkout the PR branch directly (do NOT create a new branch off it) |
There was a problem hiding this comment.
Same grammar issue here: "Checkout" should be "Check out" in the example comment.
| # Checkout the PR branch directly (do NOT create a new branch off it) | |
| # Check out the PR branch directly (do NOT create a new branch off it) |
.github/copilot-instructions.md
Outdated
| # Make fixes, commit, and push to the PR branch | ||
| git add . | ||
| git commit -m "Fix: Description of the change" | ||
| git push |
There was a problem hiding this comment.
The example workflow ends with an unconditional git push, which conflicts with rule #4 (“do NOT automatically push” when amending an existing PR). Consider updating the example to stop before pushing and instruct the agent to ask first, or explicitly label the example as only for the case where the user requested a push.
| # Make fixes, commit, and push to the PR branch | |
| git add . | |
| git commit -m "Fix: Description of the change" | |
| git push | |
| # Make fixes and commit to the PR branch | |
| git add . | |
| git commit -m "Fix: Description of the change" | |
| # Do NOT push automatically; ask the user before running: git push |
.github/copilot-instructions.md
Outdated
| 2. **When fixing CI or amending an existing PR, commit directly to the PR's branch** - Do NOT create a separate branch. The PR branch already IS a feature branch. Creating a new branch off a PR branch means CI won't run on the PR, defeating the purpose. Checkout the PR branch, commit, and push to it. | ||
|
|
||
| 3. **When amending an existing PR, do NOT automatically push** - After making changes to an existing PR branch, ask the user before pushing. This allows the user to review the changes locally first. Exception: If the user's instructions explicitly include pushing, proceed without asking. | ||
| 3. **Do NOT rebase, squash, or force-push** unless explicitly requested by the user. These operations rewrite git history and can cause problems for other contributors. Default behavior should be regular commits and pushes. | ||
|
|
||
| 4. **When amending an existing PR, do NOT automatically push** - After making changes to an existing PR branch, ask the user before pushing. This allows the user to review the changes locally first. Exception: If the user's instructions explicitly include pushing, proceed without asking. |
There was a problem hiding this comment.
.github/copilot-instructions.md
Outdated
| 1. **NEVER commit directly to `main`** - Always create a feature branch for your work. Direct commits to `main` are strictly prohibited. | ||
|
|
||
| 2. **Do NOT rebase, squash, or force-push** unless explicitly requested by the user. These operations rewrite git history and can cause problems for other contributors. Default behavior should be regular commits and pushes. | ||
| 2. **When fixing CI or amending an existing PR, commit directly to the PR's branch** - Do NOT create a separate branch. The PR branch already IS a feature branch. Creating a new branch off a PR branch means CI won't run on the PR, defeating the purpose. Checkout the PR branch, commit, and push to it. |
There was a problem hiding this comment.
Grammar: "Checkout" should be "Check out" when used as a verb (e.g., "Check out the PR branch...").
| 2. **When fixing CI or amending an existing PR, commit directly to the PR's branch** - Do NOT create a separate branch. The PR branch already IS a feature branch. Creating a new branch off a PR branch means CI won't run on the PR, defeating the purpose. Checkout the PR branch, commit, and push to it. | |
| 2. **When fixing CI or amending an existing PR, commit directly to the PR's branch** - Do NOT create a separate branch. The PR branch already IS a feature branch. Creating a new branch off a PR branch means CI won't run on the PR, defeating the purpose. Check out the PR branch, commit, and push to it. |
.github/copilot-instructions.md
Outdated
| 1. **NEVER commit directly to `main`** - Always create a feature branch for your work. Direct commits to `main` are strictly prohibited. | ||
|
|
||
| 2. **Do NOT rebase, squash, or force-push** unless explicitly requested by the user. These operations rewrite git history and can cause problems for other contributors. Default behavior should be regular commits and pushes. | ||
| 2. **When fixing CI or amending an existing PR, commit directly to the PR's branch** - Do NOT create a separate branch. The PR branch already IS a feature branch. Creating a new branch off a PR branch means CI won't run on the PR, defeating the purpose. Checkout the PR branch, commit, and push to it. |
There was a problem hiding this comment.
Should we remove the "push to it" language here? That seems somewhat in opposition to the suggestion of never pushing?
PureWeen
left a comment
There was a problem hiding this comment.
Review copilot comments and this one
https://github.com/dotnet/maui/pull/33963/changes#r2783062548
95f889a to
6a5a571
Compare
- Rule #2 now explicitly references rule #4 before pushing - Removes implicit 'commit there' language that contradicted the no-auto-push rule - Addresses Copilot reviewer and PureWeen's feedback about contradictory push guidance Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
6a5a571 to
a7f6913
Compare
…mprovements - Merge rule #4 (no auto-push) into rule #2 (work on PR branch) - Combine 'When amending' and 'When asked to update' into single section - Add emphasis on pause before pushing (multi-model reviewed) - Net result: fewer rules, same guidance, no ambiguity Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…es (dotnet#33963) <!-- Please let the below note in for people that find this PR --> > [!NOTE] > Are you waiting for the changes in this PR to be merged? > It would be very helpful if you could [test the resulting artifacts](https://github.com/dotnet/maui/wiki/Testing-PR-Builds) from this PR and let us know in a comment if this change resolves your issue. Thank you! ## Description Adds rule dotnet#2 to the Git Workflow section in copilot-instructions.md: **When fixing CI or amending an existing PR, commit directly to the PR branch.** Do not create a separate branch off a PR branch — the PR branch already IS a feature branch. Creating a new branch means CI will not run on the PR, defeating the purpose. Also adds a code example showing the correct workflow (`gh pr checkout` → commit → push). ## Motivation Copilot CLI was incorrectly creating separate branches when asked to fix CI on existing PRs, because rule dotnet#1 ("never commit to main, create a feature branch") was being over-generalized to PR branches. This caused wasted time since CI only runs on PR branches. --------- Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Co-authored-by: Shane Neuville <shneuvil@microsoft.com>
Note
Are you waiting for the changes in this PR to be merged?
It would be very helpful if you could test the resulting artifacts from this PR and let us know in a comment if this change resolves your issue. Thank you!
Description
Adds rule #2 to the Git Workflow section in copilot-instructions.md:
When fixing CI or amending an existing PR, commit directly to the PR branch. Do not create a separate branch off a PR branch — the PR branch already IS a feature branch. Creating a new branch means CI will not run on the PR, defeating the purpose.
Also adds a code example showing the correct workflow (
gh pr checkout→ commit → push).Motivation
Copilot CLI was incorrectly creating separate branches when asked to fix CI on existing PRs, because rule #1 ("never commit to main, create a feature branch") was being over-generalized to PR branches. This caused wasted time since CI only runs on PR branches.