Skip to content
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.

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

Content-preserving release content TOML #1457

Open
BD103 opened this issue Jun 26, 2024 · 2 comments
Open

Content-preserving release content TOML #1457

BD103 opened this issue Jun 26, 2024 · 2 comments
Labels
A-Build-System C-Automation Tools to make repetitive tasks easier C-Feature A new feature, making something new possible D-Modest

Comments

@BD103
Copy link
Member

BD103 commented Jun 26, 2024

Problem

There are a few issues in how we generate the initial templates for our release notes and migration guides. Namely, the TOML manifests (_guides.toml and _release-notes.toml) do not preserve user edits. This is an issue because PRs are frequently merged, cut, or edited in ways that our current tooling just skips over.

For an example, in #1340 some migration guides were deleted. In #1366 when I re-ran the tool to synchronize new changes the deleted guides were re-generated, so I had to manually delete them. This is especially challenging due to our release candidate process, which encourages frequent synchronizations of migration guides.

Solution

Create a data-model for _guides.toml and _release-notes.toml within generate-release using serde, so we can serialize and deserialize the data. (For context, we use string templates right now.) With this, we can use toml_edit to edit the existing manifest TOMLs without overwriting them.

With toml_edit, we can load any existing TOML file and edit it as we wish, while still preserving order and comments. The new system will try to load an existing _guides.toml or _release-notes.toml if it exists and edit it instead.

In the following sections I'll lay out the strategies to handling different kinds of changes. I'll use the bird emoji (🐦) to represent migration guides and the notebook emoji (📓) to represent release notes.

Merged PRs (🐦📓)

At the top of each TOML file will be the [manifest] table with the exclude property, which lists the PR numbers that are to be skipped.

[manifest]
exclude = ["12038", "12512"]

[[guides]]
# ...

In order to merge multiple PRs together, you must add all but one of them to the exclude list. The one that is not excluded will contain the merged content. (In the future, the [[guides]].url value could be updated to accept a list of URLs instead of only one.)

Reordered PRs (🐦📓)

With toml_edit, order is automatically preserved. Nothing else needs to be done in this circumstance.

Changed titles (🐦📓)

generate-release will never try to overwrite existing titles, even if they differ from the PR title on Github. You may edit this as you please.

Changed URL (🐦📓)

This should never be done, as this is how generate-release identifies which PR corresponds to which entry.

Changed filename (🐦📓)

This should also never be done. The filenames are not public-facing, and thus not worth the time renaming. generate-release probably will not overwrite this value for efficiency's sake, but you are still discouraged from changing it.

Changed area (🐦)

This should not be changed in _guides.toml, but instead the tags should be modified on the original Github PR. The tool should then automatically pick up new tags and overwrite existing areas.

This may be an issue if you merge PRs with different tags. In this case, I'd recommend just leaving them separate. If they affect different areas of Bevy, should they really be combined together?

Changed author (📓)

Like titles, this field may be modified at will without being overwritten. The author field is calculated by finding the authors of all commits in that PR, even really small changes like typo suggestions and whitespace. In this case, it is better to remove that person's name from the author field so that others do not approach them with questions about the PR that they might not be able to answer.

Cut PRs (🐦📓)

In most cases you should remove the C-Breaking-Change or C-Needs-Release-Notes label on Github, then re-generate the manifest. In rare scenarios you may employ the exclude field instead, but you should add a comment on why you didn't just remove the label.

@BD103 BD103 added C-Feature A new feature, making something new possible A-Build-System C-Automation Tools to make repetitive tasks easier D-Modest labels Jun 26, 2024
@BD103
Copy link
Member Author

BD103 commented Jun 26, 2024

I apologize for the word-wall of an issue, I'll go over this tomorrow and try to make it more digestible. :)

@alice-i-cecile
Copy link
Member

#1354 is related, and arguably part of this :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-Build-System C-Automation Tools to make repetitive tasks easier C-Feature A new feature, making something new possible D-Modest
Projects
None yet
Development

No branches or pull requests

2 participants