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

Update the "Release Checklist" in the Pull Request template #325

Open
9 tasks
davidhassell opened this issue Apr 30, 2021 · 11 comments
Open
9 tasks

Update the "Release Checklist" in the Pull Request template #325

davidhassell opened this issue Apr 30, 2021 · 11 comments
Labels
GitHub Improvement to how we use GitHub for this repository

Comments

@davidhassell
Copy link
Contributor

Hello,

I would like to suggest a couple of changes to the Pull Request template.

The release checklist currently has:

  • Authors updated in cf-conventions.adoc?
  • Next version in cf-conventions.adoc up to date? Versioning inspired by SemVer.
  • history.adoc up to date?
  • Conformance document up-to-date?

I propose removing two items, changing it to

  • Authors updated in cf-conventions.adoc?
  • Conformance document up-to-date?

These changes reflect the fact that

  • The version may or may not have already been incremented, and in any case it is beyond the scope of the PR.
  • The history entry should include the date when the PR was merged, which is unknown to the proposer. The history should instead be updated by the merger, and in any event the history checked prior to a release.

Perhaps we chould have two checklists - one for the Author and one for the Merger - that might look like:

Author checklist

  • Authors updated in cf-conventions.adoc?
  • Conformance document up-to-date?

Merger checklist

  • history.adoc up to date?

Thoughts welcome!

@davidhassell davidhassell added the GitHub Improvement to how we use GitHub for this repository label Apr 30, 2021
@erget
Copy link
Member

erget commented Apr 30, 2021

@davidhassell you have my support, since we're merging continually and minting releases periodically this makes sense to me.

Rather than removing "history.adoc up to date?" we might consider using "history.adoc updated?" in order to ensure that we note the changes, and then when creating a new release we fill in the date as part of that checklist. That would ensure that the Author at least proposes the new lines in history.adoc.

@JonathanGregory
Copy link
Contributor

JonathanGregory commented Apr 30, 2021 via email

@davidhassell
Copy link
Contributor Author

Sounds good, thanks. How about:

Author checklist

  • Authors updated in cf-conventions.adoc?
  • Conformance document up-to-date?
  • Related issues registered in history.adoc, with placeholders for the dates?

Merger checklist

  • Set dates for new history.adoc entries?

@erget
Copy link
Member

erget commented May 4, 2021

@davidhassell the author checklist looks correct for me. From my side I have 1 more question about the merger checklist - does history.adoc get its date updated when merged, or when tagged? My understanding is that we might merge multiple PRs into a tag-ready branch, but then only tag it when we want to make a new release, meaning that the tag would pull in changes from multiple PRs.

@davidhassell
Copy link
Contributor Author

Hi @erget,

I don't know. I had thought that we would merge PRs into master at the time of their acceptance, so that future PRs for the same next release can build on the accepted changes.

I may well have misunderstood things, apologies if so, but in any event, if we say "Set the date when merged into master for new history.adoc entries", are we covered?

@erget
Copy link
Member

erget commented May 4, 2021

Hi @davidhassell as secretary I consider you the oracle of minting releases, so I'm sure you're right. The thought that I had was, if we're merging continuously into master, and then only tagging at certain points on master, then we'd be updating the dates in the history superfluously, no? Which wouldn't be bad, but life is made of tiny problems to solve, so we could perhaps do without and do it as part of a tagging checklist.

It might be easier to chat about it this afternoon when we meet at 2 anyway :)

@davidhassell
Copy link
Contributor Author

Hi @erget,

From what you describe, should the dates always be the date of minting a release, rather than the date merging. That could clear things up? I think a quick chat later would be good!

I'd also like to add a merger item for the data model:

Merger checklist

  • <something about dates?>
  • Data model has been considered in the related issue(s)?

In most cases cases the data model is not impacted, but I think that it would be good to see a comment stating that it has been thought about in the related issue, rather than relying on a default assumption that "no mention = no impact".

@davidhassell davidhassell changed the title Update the "Release Cheklist" in the Pull Request template Update the "Release Checklist" in the Pull Request template May 4, 2021
@ethanrd
Copy link
Member

ethanrd commented May 4, 2021

Hi @davidhassell, @erget, all - In the current history.doc, the release/version history is hard to scan while the change/issue history is easy (though verbose) to scan. Whether dates are only on releases or on both releases and change/issue, I would like to suggest we move towards a "Release - " header with all the changes/issues added in that version listed under that header. Something like:

Release 1.x - dd Month year

  • Issue #nnn: summary text (or title)
  • Issue #mmm: summary text/title

@erget
Copy link
Member

erget commented May 5, 2021

@ethanrd I agree with that approach. We could group the stuff with headers. In a lot of projects I work on I take inspiration from Keep a Changelog, so my changelogs (or in our case the history file) look like this:

[2.10.0]

Added

  • Add UMARF plugins to installer constructor and pipelines (#823)
  • Provided descriptions for OpenAPI specs definitions (#808)
  • Deploy native to HRIT conda package to EUMETSAT conda repository (#803)
  • ...

Changed

  • Adapt DTWS dockerfile to execute commands as user (#853)
  • Temporarily remove support for MTG IRS L1 as format is being updated (#840)
  • Removing also the processing-dir (if any) when a customisation is deleted by user or administrator (#837)
  • ...

Removed

  • Removed check_olda_cache option as Data Store input products are now downloaded in temporary processing_dir (#775)

Fixed

  • Properly parse content disposition as products downloaded via GUI on chrome contain start and trail underscore (#869)
  • Ensure netcdf-satellite output files can correctly be downloaded from GUI via DTWS (#865)
  • Fix 50-x service pages configuration (#854)
  • ...

If we want to keep continuity we could add a merge date on each bullet point and a tag date on the release line, that way one wouldn't need the git history to reconstruct when a release was made.

@JonathanGregory
Copy link
Contributor

history.adoc has since reformatted somewhat as discussed in this issue 2.5 years ago, but the original proposal to update the "release checklist" hasn't been implemented yet.

@JonathanGregory
Copy link
Contributor

Please see #369, which may lead to some changes that might affect what goes in the pull request template.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
GitHub Improvement to how we use GitHub for this repository
Projects
None yet
Development

No branches or pull requests

4 participants