Enhance integration issue fixing guidelines#1536
Conversation
Expanded guidelines for fixing issues during and after integration review, including workflow and commit practices.
✅ Deploy Preview for moodledevdocs ready!Built without sensitive environment variables
To edit notification comments on pull requests, go to your Netlify project configuration. |
There was a problem hiding this comment.
Pull request overview
Updates the Moodle integration process documentation to clarify how developers should respond to feedback and address problems at different stages of integration, aiming to make integrator review/testing smoother and more traceable.
Changes:
- Replaces the brief “don’t rewrite history” guidance with a more structured workflow split into “before integration” vs “after integration”.
- Adds guidance on using fixup-style follow-up commits during integration review, with examples.
- Adds a post-integration “create a new branch + comment on the issue” workflow for fixes found during integration testing.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| As a general rule, this means that if your issue has entered the 'in integration review' stage of the development process, please only add new commits on top of your existing commits. There are circumstances when your issue will be 'in integration review' but not merged (and thus possible to squash changes) but if in any doubt, please add new commits and ask the integrator to squash your changes for you. | ||
| ### Fixing issues before integration (during integration review) | ||
|
|
||
| Once an issue has entered the "In integration review" stage, you must not modify the existing commit history of your branch. |
| Add new commits on top of the existing branch, using FIXUP! commit messages wherever appropriate. These commits make it clear which changes are follow-ups to earlier commits. | ||
|
|
||
| For example: | ||
|
|
||
| ```console | ||
| commit a1b2c3d First implementation of feature | ||
| commit d4e5f6a Add missing unit tests | ||
| commit 12ab34f FIXUP! Address review feedback: fix coding style | ||
| commit 56cd78e FIXUP! Adjust logic after integration testing |
|
|
||
| - Do not amend commits | ||
| - Do not squash commits | ||
| - Do not rebase or force-push updated history |
|
|
||
| #### 1. Create a new branch for the fix | ||
|
|
||
| Use the format: MDL-12345-main-fix |
|
|
||
| Use the format: MDL-12345-main-fix | ||
|
|
||
| Base this branch on the integrated branch (i.e., the branch as it exists in the integration repository). |
| #### 2. Add your fix as a new commit | ||
|
|
||
| - Do not rebase or modify the previously integrated branch | ||
| - Do not force push changes to the old development branch |
|
Thanks Simey for proposing this. I think we need something liike this, but I think it needs a bit more work. By the way, did you know about https://moodledev.io/general/development/abc/fixups, which I wrote with help from Andrew L recently. - I kind of think that the detailed explanation/tutorial of what to do is better outside the main integration process. Of course, if you can improve that abc page, please do. I agree developers should provide fixup commits. However, before integration, I think it is better to keep the merge branches clean, and ready to merge. Therefore, my suggetion was that developers push the branch with separate fixup commits with a different name, and link to that in the comments, then rebase and push with the integration branch name. Of course, after merge, extra fixes need to be separate commits - but then they need proper commit comments, not ones started fixup! Just a suggestion. I hope it is useful. Tim. |
This pull request updates the integration workflow documentation to clarify the correct process for handling fixes during and after integration review. The new instructions help developers avoid rewriting history in ways that complicate the integrator's work, and provide clear steps for submitting follow-up fixes.
Integration workflow clarification: