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

Updates to RELEASE.md #2195

Merged
merged 9 commits into from
Jun 24, 2022
Merged

Updates to RELEASE.md #2195

merged 9 commits into from
Jun 24, 2022

Conversation

pstibrany
Copy link
Member

@pstibrany pstibrany commented Jun 23, 2022

What this PR does

This PR updates RELEASE.md documentation for maintainers doing releases.

  • In first commit it just breaks long lines, and adds more releases.
  • Second commit expands our release notes: adds contributor stats and new contributors, and explains how to find that.
  • Third commit clarifies procedure for doing cherry-picks, that preserves contributor stats in Github but also adds extra traceability to commit messages by linking the original commit message.
  • (Additional commits will reflect review feedback 😄 and inevitable linter complaints)

Since this is internal document for maintainers and not end users, I don't consider it as "documentation", hence not engaging docs team.

Checklist

  • [na] Tests updated
  • [na] Documentation added
  • [na] CHANGELOG.md updated - the order of entries should be [CHANGE], [FEATURE], [ENHANCEMENT], [BUGFIX]

@pstibrany pstibrany requested a review from pracucci June 23, 2022 09:59
@pstibrany pstibrany changed the title Release updates Updates to RELEASE.md Jun 23, 2022
@pstibrany pstibrany marked this pull request as ready for review June 23, 2022 09:59
Signed-off-by: Peter Štibraný <[email protected]>
Copy link
Contributor

@56quarters 56quarters left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Haven't done a release yet so I can't comment on the actual steps but these changes LGTM!

Copy link
Contributor

@jhesketh jhesketh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me, just some nits inline.

RELEASE.md Outdated Show resolved Hide resolved
RELEASE.md Outdated Show resolved Hide resolved
RELEASE.md Outdated Show resolved Hide resolved
RELEASE.md Outdated Show resolved Hide resolved
Signed-off-by: Peter Štibraný <[email protected]>
Copy link
Collaborator

@pracucci pracucci left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for working on this! Few comments.

Also, we should document to write the release notes in our doc too. See #1848 for reference. We may also require a mandatory review from a product manager (e.g. @09jvilla).

RELEASE.md Outdated
The release shepherd is responsible for an entire minor release series, meaning all pre- and patch releases of a minor release.
The process formally starts with the initial pre-release, but some preparations should be made a few days in advance.

- We aim to keep the `main` branch in a working state at all times. In principle, it should be possible to cut a release
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[not a blocker, but agreeing on some consistent will help long term maintaneability] I'm not sure this line break splitting follows docs squad recommendation. To my understanding there should be 1 full sentence per line, to help code reviews (so that with GitHub suggestions you can easily rewrite 1 sentence at a time). @jdbaldry am I right or wrong?

Copy link
Member Author

@pstibrany pstibrany Jun 24, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I typically stick to 1 sentence / line format (unless sentences are very short, or too long – then I break them), but got carried away in this list. Will fix it.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am definitely in favor of one line per sentence for easier line based diffing. I'm not yet certain that this is a strong enough argument to move from personal preference to enforced style though.

RELEASE.md Outdated Show resolved Hide resolved
RELEASE.md Outdated
- You can find the numbers by doing compare like `https://github.com/grafana/mimir/compare/mimir-2.0.0...mimir-2.1.0` from previous Mimir release. (Note the syntax: `.../compare/<old ref>...<new ref>`, yes there are three periods).
- Number of commits = "contributions", number of contributors = "authors".
- As an example, for [current HEAD on commit 017a738](https://github.com/grafana/mimir/compare/mimir-2.1.0...017a738e94) shows 185 contributions and 36 authors since Mimir 2.1.0.)
- Most interesting new features for users. Use your judgement.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would be more explicit here. Look at https://github.com/grafana/mimir/releases/tag/mimir-2.1.0. In Mimir so far we had "Features and enhancements", "Upgrade considerations", and "Bug fixes". I would keep that format, cause it worked fine so far.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've added new section on writing release notes document, and at this place I just refer to it now:

   - After contributor stats, please include content of the release notes document [created previously](#write-release-notes-document).

Signed-off-by: Peter Štibraný <[email protected]>
Signed-off-by: Peter Štibraný <[email protected]>
Signed-off-by: Peter Štibraný <[email protected]>
@pstibrany
Copy link
Member Author

Thanks for working on this! Few comments.

Also, we should document to write the release notes in our doc too. See #1848 for reference. We may also require a mandatory review from a product manager (e.g. @09jvilla).

Thanks Marco for review and suggestions. I've applied your feedback. I've added new section on writing release notes document.

Copy link
Collaborator

@pracucci pracucci left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks!

Copy link
Contributor

@jhesketh jhesketh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm 👍

@pstibrany pstibrany merged commit 567e0cc into main Jun 24, 2022
@pstibrany pstibrany deleted the release-updates branch June 24, 2022 12:04
@pstibrany
Copy link
Member Author

Thank you all for reviews and feedback!

masonmei pushed a commit to udmire/mimir that referenced this pull request Jul 11, 2022
* Add more releases, split long lines for easier editing.

Signed-off-by: Peter Štibraný <[email protected]>

* Added section on creating release on GitHub with contributor stats and new contributors section.

Signed-off-by: Peter Štibraný <[email protected]>

* Added part about cherry-picking with -x and squash & merge.

Signed-off-by: Peter Štibraný <[email protected]>

* Move opening PR to text.

Signed-off-by: Peter Štibraný <[email protected]>

* What's more in life than happy linter?

Signed-off-by: Peter Štibraný <[email protected]>

* Apply feedback from review.

Signed-off-by: Peter Štibraný <[email protected]>

* Fix.

Signed-off-by: Peter Štibraný <[email protected]>

* Address review feedback.

Signed-off-by: Peter Štibraný <[email protected]>

* Address review feedback.

Signed-off-by: Peter Štibraný <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants