Improve Copilot instructions - #4142
mbg merged 3 commits into
Conversation
Sorry, something went wrong.
There was a problem hiding this comment.
Copilot review overview
🟡 Changes recommended
The instruction at line 16 ends mid-sentence and needs clarification.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review tier: Lite
Findings: 1
| Severity | Finding |
|---|---|
| .github/copilot-instructions.md — Complete the truncated follow-up guidance |
Documentation-only update to repository-specific Copilot review guidance.
Changes:
- Documents repository structure, generated code, and pr-checks.
- Adds release-process guidance.
- Adjusts nested Markdown formatting.
Review note: Complete the instruction at line 16 so follow-up PR handling is unambiguous.
| File | Summary |
|---|---|
| .github/copilot-instructions.md | Updated Copilot instructions and review guidance. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Sorry, something went wrong.
mario-campos
left a comment
There was a problem hiding this comment.
Nice improvement. I added a few suggestions that I think would help make the points more explicit and direct.
Sorry, something went wrong.
| ## Review instructions | ||
|
|
||
| - When wording review comments, be helpful and friendly. Assume that the PR author has written the code with the best of intentions. Word your comments constructively as suggestions for improvements. Do not word suggestions as commands. | ||
| - If you want to comment on a change that you believe will fail a CI check, do not present the CI failure you expect as a fact. Instead, write that you think a change "may" lead to a failure in CI. Suggest that, if such a failure manifests, the changes you are commenting on may be the place responsible for the failure and are worth looking at. |
There was a problem hiding this comment.
| - If you want to comment on a change that you believe will fail a CI check, do not present the CI failure you expect as a fact. Instead, write that you think a change "may" lead to a failure in CI. Suggest that, if such a failure manifests, the changes you are commenting on may be the place responsible for the failure and are worth looking at. | |
| - If you want to comment on a change that you believe will fail a CI check, you may state that, but do not make any definitive unsubstantiated statements about CI pass/fail. Instead, write that you think a change "may" lead to a failure in CI. Suggest that, if such a failure manifests, the changes you are commenting on may be the place responsible for the failure and are worth looking at. |
Sorry, something went wrong.
There was a problem hiding this comment.
This mostly seems like a more verbose version of what I wrote. Did you have a particular concern about the existing wording that you wanted to address here?
Sorry, something went wrong.
|
@mario-campos Thanks for the review! I have pushed a commit which I hope addresses your comments. I have skipped some of the word changes and tried to avoid more duplication. In general though, I think it makes sense to merge the changes without too much discussion about the wording of the instructions. We can see what does or doesn't work in practice once the changes are merged, and then address issues if we notice persistent problems. |
Sorry, something went wrong.
Some changes to copilot-instructions.md based on some observations of Copilot's reviews since the last change was made.
Risk assessment
For internal use only. Please select the risk level of this change:
Which use cases does this change impact?
Environments:
How did/will you validate this change?
If something goes wrong after this change is released, what are the mitigation and rollback strategies?
How will you know if something goes wrong after this change is released?
Are there any special considerations for merging or releasing this change?
Merge / deployment checklist