HonkingGoose
commented
2 weeks ago Accept one-time "bad" thing for simplicity
We should not proactively edit every existing PR and cause a lot of edits, but if we are already editing an existing PR then we should add these delimiters. If editing a PR then we should retain any content before or after.
This is your judgment call of course, but I think it's okay for Renovate to update all the PRs at once. If we accept that one-time "bad thing", we get:
- Fewer lines of code
- Fewer bugs, due to less lines of code where bugs can "hide"
- Simpler tests
- Simpler behavior
- Less code to review/maintain
Options
I think we have these options:
- Always add delimiters
- Conditional add delimiters
- Something else?
Always add delimiters
This seems easy:
- Write tests for the add delimiter feature
- Write code to add delimiters
Conditional add delimiters
This seems hard:
- Write tests for the happy path for the delimiter feature
- Write code to add delimiters
- Write tests for the difficult paths, where Renovate should skip adding delimiters
- Write code for the difficult paths
- Write code to allow updating open PRs, when Renovate is "touching" the PR for some other reason
Something else?
I don't know if there are better options than the things listed above. :smile:
Describe the proposed change(s).
We should start adding
<!--renovate-pr-body-start-->before our content and<!--renovate-pr-body-end-->after it, and ignoring any content before or after that. If either tag is missing then we assume that the start or end of the content is the start or end of the PR body itself.We should not proactively edit every existing PR and cause a lot of edits, but if we are already editing an existing PR then we should add these delimiters. If editing a PR then we should retain any content before or after.
For platforms which don't support hidden html comments, we should strip this content out, which means we don't support PR body editing on those platforms.