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

Changelog for v2.8.0rc2 #1959

Merged
merged 20 commits into from
Mar 17, 2023
Merged

Changelog for v2.8.0rc2 #1959

merged 20 commits into from
Mar 17, 2023

Conversation

remi-kazeroni
Copy link
Contributor

@remi-kazeroni remi-kazeroni commented Mar 8, 2023

Description

Changelog for v2.8.0rc2

Related to #1951

Link to documentation: https://esmvaltool--1959.org.readthedocs.build/projects/ESMValCore/en/1959/changelog.html


Before you get started

Checklist

It is the responsibility of the author to make sure the pull request is ready to review. The icons indicate whether the item will be subject to the 🛠 Technical or 🧪 Scientific review.


To help with the number pull requests:

@remi-kazeroni remi-kazeroni added this to the v2.8.0 milestone Mar 8, 2023
@remi-kazeroni remi-kazeroni self-assigned this Mar 8, 2023
@codecov
Copy link

codecov bot commented Mar 8, 2023

Codecov Report

Merging #1959 (cc0a6d7) into main (a86c92b) will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##             main    #1959   +/-   ##
=======================================
  Coverage   92.79%   92.79%           
=======================================
  Files         236      236           
  Lines       12437    12437           
=======================================
  Hits        11541    11541           
  Misses        896      896           

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

@valeriupredoi
Copy link
Contributor

valeriupredoi commented Mar 16, 2023

doc test failing because of this /home/docs/checkouts/readthedocs.org/user_builds/esmvalcore/checkouts/1959/doc/changelog.rst:: ERROR: Anonymous hyperlink mismatch: 1 references but 0 targets. Readthedocs builder is as descriptive as a cow applying for a job at NASA

@remi-kazeroni
Copy link
Contributor Author

Yeah, I did some copy-pasting in the last commit without formatting all the links properly. Will fix that now and make this rfr. We may need to give the TLT the chance to take a look. Will post on it asap.

@remi-kazeroni remi-kazeroni marked this pull request as ready for review March 16, 2023 16:13
@remi-kazeroni remi-kazeroni mentioned this pull request Mar 16, 2023
4 tasks
@remi-kazeroni
Copy link
Contributor Author

@ESMValGroup/technical-lead-development-team after quite some struggle with our machinery (see #1974), here is our changelog for v2.8. Could you please take a look and let me know if it look good to you? Don't hesitate to provide GH suggestions that I could push directly, if necessary 👍 Deadline: Friday 16:00 (CET) or please say that you need more time.

@bouweandela: do you think you could provide some text for the backward incompatibility and deprecations originating from PRs you authored? That would be very helpful for me 👍

Copy link
Contributor

@valeriupredoi valeriupredoi 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, bud! Cheers very much for doing this 🍺

Copy link
Contributor

@schlunma schlunma left a comment

Choose a reason for hiding this comment

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

Thanks @remi-kazeroni!

doc/changelog.rst Outdated Show resolved Hide resolved
Rémi Kazeroni and others added 3 commits March 17, 2023 09:54
Co-authored-by: sloosvel <45196700+sloosvel@users.noreply.github.com>
@bouweandela
Copy link
Member

Thank you for creating the changelog @remi-kazeroni. Is it OK if I push the changes directly to this branch or shall I make a pull request?

@remi-kazeroni
Copy link
Contributor Author

@bouweandela feel free to push the changes here directly. Then I'll take a look. Unless these are big changes, I hope we can agree on merging this PR today and then I could still do the release of v2.8.0rc2 on GitHub (and PyPi)

Improve backward incompatible and deprecated explanation

Use links instead of URLs

Shorten lines

Add a preprocessor section

Move more pull requests to the correct section
@bouweandela
Copy link
Member

I noticed that the Preprocessor section has gone missing but the pull requests do have the right label, maybe this is something in the script you used? I added it in the last commit.

@remi-kazeroni
Copy link
Contributor Author

Thanks for your help and suggestions @bouweandela! Looks good to me at first sight but we need to get the build fixed. Large commits and trouble to build locally can make it challenging to debug, spent my afternoon on that yesterday already 😅

I noticed that the Preprocessor section has gone missing but the pull requests do have the right label, maybe this is something in the script you used? I added it in the last commit.

When I ran draft_release_notes.py, I got an empty list for the Preprocessor section because not many PRs used that label and even if it was used, the script picks another label which comes before in the alphabetical order.

I agree with your idea of making links to refer to PRs more than once. It's very challenging not be able to cite a PR more than once when you want to highlight it and at the same time provide some guidance to users on how to handle deprecations and co.

Is there any particular reason for not ordering the PRs in ascending PR numbers in every section any more? This is done automatically with draft_release_notes.py

@remi-kazeroni
Copy link
Contributor Author

Just some thoughts for the future: I think we should really revise the way we generate the changelog. This requires too much last-minute tweaking for the RM and too many manual steps (probably more than for testing all recipes) to handle 50+ PRs with 2 or 3 labels each. What about having a changelog PR open as soon as we start a new release cycle? We could make it the responsibility of the PR merger or author to add the merged PR to the appropriate section of the changelog. They may know better where to put them than the RM. Before generating the final version of the changelog, the RM would run the script draft_release_notes.py only to check that all PRs were added. And then the RM would handle the highlights, messages to the users.

@valeriupredoi
Copy link
Contributor

Just some thoughts for the future: I think we should really revise the way we generate the changelog. This requires too much last-minute tweaking for the RM and too many manual steps (probably more than for testing all recipes) to handle 50+ PRs with 2 or 3 labels each. What about having a changelog PR open as soon as we start a new release cycle? We could make it the responsibility of the PR merger or author to add the merged PR to the appropriate section of the changelog. They may know better where to put them than the RM. Before generating the final version of the changelog, the RM would run the script draft_release_notes.py only to check that all PRs were added. And then the RM would handle the highlights, messages to the users.

I agree with @remi-kazeroni - I'd like this discussed on a TLT call - Remi, I'll open an issue about this if that's OK with you!

@bouweandela
Copy link
Member

The build fails because this undefined label: esmvaltool:read_icon, maybe I got the reference wrong?

@valeriupredoi
Copy link
Contributor

@bouweandela I don't see the warning you mention at https://readthedocs.org/projects/esmvalcore/builds/19816569/ - all I see is the pandoc warning which I fixed by pinning sphinx in #1976

@valeriupredoi
Copy link
Contributor

ah nevermind - it was out of the screen - indeed - that's another warning

@valeriupredoi
Copy link
Contributor

excellent! So we don't really really need #1976 for RTD builds - we may need it for local builds since Remi had issues with his - bet let's do it after the release. At any rate - let's get this in, @bouweandela would you care to merge, please, bud? I can do it if you don't do it within a lap at Le Mans/La Sarthe time (~4min) 😁

@remi-kazeroni
Copy link
Contributor Author

Let me take a very final look now please 👍 And yes, I would have saved a lot of time if I knew that when you click on "rdt: details" > "view raw" you actually see the full log, not the raw .rst file... 🤯 At least I learnt something!

@bouweandela
Copy link
Member

bouweandela commented Mar 17, 2023

Is there any particular reason for not ordering the PRs in ascending PR numbers in every section any more?

Some pull requests seem a lot more relevant than others to our users, so that's why I moved those closer to the top of the list in some cases. And I grouped all the multimodel ones.

I think we should really revise the way we generate the changelog. This requires too much last-minute tweaking for the RM

Indeed writing the changelog is not something that should be left to the last minute. The script is called draft release notes.py for a reason, I wrote it so I had something to start from when making a release, it was never intended to produce the final version of the release notes. Maybe we should drop the script altogether and do it like they do it for iris: as part of your pull request you write a description in the release notes, if the pull request is relevant enough for users.

@bouweandela bouweandela merged commit 007a3a1 into main Mar 17, 2023
@bouweandela bouweandela deleted the update_changelog_v280rc2 branch March 17, 2023 15:46
@bouweandela
Copy link
Member

Whoops, too quick! Sorry @remi-kazeroni

@remi-kazeroni
Copy link
Contributor Author

It's ok, we'll have a last chance to improve the formatting for the final release (see my release section...). Let's move on to rc2 release now!

@valeriupredoi
Copy link
Contributor

do it like they do it

on the Discovery Channel 🤣

Cheers guys, we are OK for now. Let's get some of these things sorted for 2.9 - at least we have now sorted the many steps for Tool in that dox PR - baby steps

remi-kazeroni pushed a commit that referenced this pull request Mar 17, 2023
Co-authored-by: sloosvel <45196700+sloosvel@users.noreply.github.com>
Co-authored-by: Bouwe Andela <b.andela@esciencecenter.nl>
@remi-kazeroni remi-kazeroni mentioned this pull request Mar 20, 2023
7 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants