Skip to content

Conversation

@valeriupredoi
Copy link
Contributor

@valeriupredoi valeriupredoi commented Dec 2, 2022

@ESMValGroup/technical-lead-development-team these are the planned changes for the bugfix release that we talked about earlier today: they include:

Please have a look - this PR will be merged (note the target for merge: release v2.7.0 branch) only when we agree all is fine, and then after I change the version number.

Steps to follow

valeriupredoi and others added 30 commits October 14, 2022 12:17
…elease (#1756)

* updating changelog in main to reflect changelog from release,merge postrelease

* final changelog for release, complete and locked

* run GA tests on releae branch for the last time just to be super sure

* remove call to run GA tests on release branch, godspeed, little fellow

* update the cff

* last bit of changelog
* fix anaconda badge

* correct repo in badge
Updating Linux condalock file

Co-authored-by: valeriupredoi <valeriupredoi@users.noreply.github.com>
Updating Linux condalock file

Co-authored-by: valeriupredoi <valeriupredoi@users.noreply.github.com>
Update condalock file
Automatic Pull Request.

Co-authored-by: valeriupredoi <valeriupredoi@users.noreply.github.com>
<!--
    Thank you for contributing to our project!

Please do not delete this text completely, but read the text below and
keep
items that seem relevant. If in doubt, just keep everything and add your
    own text at the top, a reviewer will update the checklist for you.

-->

## Description

<!--
Please describe your changes here, especially focusing on why this pull
    request makes ESMValCore better and what problem it solves.

Before you start, please read our contribution guidelines:
https://docs.esmvaltool.org/projects/ESMValCore/en/latest/contributing.html

    Please fill in the GitHub issue that is closed by this pull request,
    e.g. Closes #1903
-->
This PR allows to compute the sea ice extent as a derived variable in
order to reduce the amount of saved data during the preprocessing
Closes #1693 

Link to documentation:

***

## [Before you get
started](https://docs.esmvaltool.org/projects/ESMValCore/en/latest/contributing.html#getting-started)

- [x] [☝ Create an
issue](https://github.com/ESMValGroup/ESMValCore/issues) to discuss what
you are going to do

##
[Checklist](https://docs.esmvaltool.org/projects/ESMValCore/en/latest/contributing.html#checklist-for-pull-requests)

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][1] or [🧪 Scientific][2] review.

<!-- The next two lines turn the 🛠 and 🧪 below into hyperlinks -->
[1]:
https://docs.esmvaltool.org/en/latest/community/review.html#technical-review
[2]:
https://docs.esmvaltool.org/en/latest/community/review.html#scientific-review

- [x] [🧪][2] The new functionality is [relevant and scientifically
sound](https://docs.esmvaltool.org/projects/ESMValCore/en/latest/contributing.html#scientific-relevance)
- [x] [🛠][1] This pull request has a [descriptive title and
labels](https://docs.esmvaltool.org/projects/ESMValCore/en/latest/contributing.html#pull-request-title-and-label)
- [x] [🛠][1] Code is written according to the [code quality
guidelines](https://docs.esmvaltool.org/projects/ESMValCore/en/latest/contributing.html#code-quality)
~- [ ] [🧪][2] and [🛠][1]
[Documentation](https://docs.esmvaltool.org/projects/ESMValCore/en/latest/contributing.html#documentation)
is available~
- [x] [🛠][1] [Unit
tests](https://docs.esmvaltool.org/projects/ESMValCore/en/latest/contributing.html#tests)
have been added
- [x] [🛠][1] Changes are [backward
compatible](https://docs.esmvaltool.org/projects/ESMValCore/en/latest/contributing.html#backward-compatibility)
~- [ ] [🛠][1] Any changed [dependencies have been added or
removed](https://docs.esmvaltool.org/projects/ESMValCore/en/latest/contributing.html#dependencies)
correctly~
- [x] [🛠][1] The [list of
authors](https://docs.esmvaltool.org/projects/ESMValCore/en/latest/contributing.html#list-of-authors)
is up to date
- [x] [🛠][1] All [checks below this pull
request](https://docs.esmvaltool.org/projects/ESMValCore/en/latest/contributing.html#pull-request-checks)
were successful

***

To help with the number pull requests:

- 🙏 We kindly ask you to
[review](https://docs.esmvaltool.org/en/latest/community/review.html#review-of-pull-requests)
two other [open pull
requests](https://github.com/ESMValGroup/ESMValCore/pulls) in this
repository
…nd reinstalling deps with pip at readthedocs builds (#1786)
Co-authored-by: Klaus Zimmermann <klaus.zimmermann@smhi.se>
Co-authored-by: valeriupredoi <valeriupredoi@users.noreply.github.com>
Co-authored-by: Manuel Schlund <32543114+schlunma@users.noreply.github.com>
Co-authored-by: Valeriu Predoi <valeriu.predoi@gmail.com>
Co-authored-by: Remi Kazeroni <remi.kazeroni@dlr.de>
)

Co-authored-by: Bouwe Andela <b.andela@esciencecenter.nl>
…e saving files (#1837)

Co-authored-by: Bouwe Andela <b.andela@esciencecenter.nl>
@valeriupredoi valeriupredoi changed the base branch from release_270stable to main December 5, 2022 15:56
@valeriupredoi
Copy link
Contributor Author

@bouweandela so this is how it looks if we really want to merge this into main - it's a bomb 💣 I honestly don't know how we'd get about this, as I see it we have two alternatives:

  • we either create a 2.7.1 off the latest main but that means we have to test run all the recipes
  • or, we keep the local changes before 3b0f1f6 and we create a bugfix release candidate off this branch, then we either don't update the version in main since that's already at 2.7.1dev by default, or we update the version manually but don't release anything

Opinions? 🍺

Oh and BTW the test_multimodel in sample data is an absolute nightmare to prune, since it contains a lot of stuff that deals with updates by @schlunma in main - those are beautiful, but are causing the poopstorm wrt conflicts

@schlunma
Copy link
Contributor

schlunma commented Dec 6, 2022

With all the changes since 2.7.0 (especially in the multimodel module and its tests) I guess merging this branch back into main without changes is not possible (some tests from 2.7.0 fail in the current main, and some tests from the current main fail in the 2.7.0 branch).

Is there a way to hardcode the version number somehow? If we don't merge this back into main, will this also mess up the version numbers of future releases?

@valeriupredoi
Copy link
Contributor Author

With all the changes since 2.7.0 (especially in the multimodel module and its tests) I guess merging this branch back into main without changes is not possible (some tests from 2.7.0 fail in the current main, and some tests from the current main fail in the 2.7.0 branch).

exactly - it's a bit of a no-go either way; we fought ourselves into a corner, and we won't get away without serious recipes testing if we want to keep the things off main

Is there a way to hardcode the version number somehow? If we don't merge this back into main, will this also mess up the version numbers of future releases?

No, it shouldn't mess it up; plus the current version of main is already at 2.7.1devXXX

I'd very much appreciate a bit of feedback from @bouweandela before I revert commits and prepare a release with the changes before 3b0f1f6 - also bit of feedback from you Manu after I revert the commits would be greatly appreciated 🍺

@bouweandela
Copy link
Member

We need something that can be merged back into main to get the versioning working properly. However, we can of course add commits to that to undo any unwanted changes after making the release but before merging it back into main. This will make a bit of a mess of our version history, but we if make an effort to keep the git history somewhat clean it should hopefully not be too bad.

The (least bad) way forward I see is the following:

  • start by branching off the v2.7.0 tag
  • add 1 commit with a clear description per feature that will be added to v2.7.1, keeping in mind that we want minimal differences with main
  • create the release
  • add 1 commit to undo any changes we do not want in main (such as pinning the mypy version)
  • merge the release branch back into main

@valeriupredoi
Copy link
Contributor Author

good call @bouweandela - I did that before 3b0f1f6 (even if my commits are a bit too many) - I will revert the commits later than that commit, keep this branch for reference and will work on a new branch (off the 27 release) and include the changes from here on that branch with more condensed and descriptive commits

@valeriupredoi valeriupredoi changed the base branch from main to release_270stable December 8, 2022 13:12
@valeriupredoi
Copy link
Contributor Author

OK @bouweandela so these are the changes we need to get in the bugfix release branch, re-based the PR to the stable release branch, things look good (apart from Codacy which is losing its plot). We'd also need your change from #1854

@bouweandela
Copy link
Member

To make the release, we'll need to create a branch called v2.7.x and add the commits that we want in the bugfix release. Do you want to manually create meaningful commits? Or would you prefer to make pull requests so we can squash-merge them and get a clean git history like that?

@valeriupredoi
Copy link
Contributor Author

how about we rename release_270stable to 2.7.x then squash merge this in it, then we up the version there to 2.7.1 and create a release off it?

@bouweandela bouweandela changed the base branch from release_270stable to v2.7.x December 8, 2022 13:59
@bouweandela
Copy link
Member

After some discussion offline, @valeriupredoi and I decided to make one pull request per added feature for v2.7.1 against the v2.7.x branch.

@valeriupredoi
Copy link
Contributor Author

indeed! and I will close this once all the PRs to 2.7.x have been initiated. 🐻 with me now...

@valeriupredoi
Copy link
Contributor Author

valeriupredoi commented Dec 8, 2022

@bouweandela
Copy link
Member

Would it be possible to include #1838 as well? That will make sure everyone has an up-to-date default ESGF configuration again when they install the tool.

@valeriupredoi
Copy link
Contributor Author

Would it be possible to include #1838 as well? That will make sure everyone has an up-to-date default ESGF configuration again when they install the tool.

I think we're good to include that too, but unfortunately am very hungry now, so I'll have dinner, will do that tomorrow, after you've plugged in all the other component PRs, man 🍺

@valeriupredoi valeriupredoi marked this pull request as draft December 11, 2022 11:38
@valeriupredoi
Copy link
Contributor Author

changed this to Draft so there is really no accidental push to v2.7.x (which I am also gonna protect via commit restrictions only for myself and @bouweandela ). v2.7.x looks to be just about ready for the bugfix release anyway, so we'll close this soon. Very many thanks for your review work @bouweandela 🍺

@valeriupredoi
Copy link
Contributor Author

we now have a bugfix release https://pypi.org/project/ESMValCore/2.7.1/ so I'm closing this 👍

@valeriupredoi valeriupredoi deleted the release_271bugfix branch December 12, 2022 14:33
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.

6 participants