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

Pin importlib_metadata <8 #3700

Merged
merged 4 commits into from
Jul 1, 2024
Merged

Pin importlib_metadata <8 #3700

merged 4 commits into from
Jul 1, 2024

Conversation

ehogan
Copy link
Contributor

@ehogan ehogan commented Jul 1, 2024

Description


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 of pull requests:

@ehogan ehogan self-assigned this Jul 1, 2024
@ehogan ehogan added this to the v2.11.0 milestone Jul 1, 2024
@ehogan ehogan marked this pull request as ready for review July 1, 2024 09:41
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.

excellent, cheers muchly @ehogan 🍺

@bouweandela
Copy link
Member

Is this needed for the main branch? I understood that importlib_metadata version 8 is incompatible with esmpy < 8.6, but in the main branch esmpy is pinned to >= 8.6 since #3643.

@ehogan ehogan changed the base branch from main to v2.11.x July 1, 2024 16:20
@ehogan
Copy link
Contributor Author

ehogan commented Jul 1, 2024

Is this needed for the main branch? I understood that importlib_metadata version 8 is incompatible with esmpy < 8.6, but in the main branch esmpy is pinned to >= 8.6 since #3643.

Ah, yes, since importlib_metadata will be removed from ESMValCore after this release (see my comment on #3699), it makes sense to add this change to the release branch. I have updated the PR to target the release branch 👍

@bouweandela bouweandela merged commit c0ab625 into v2.11.x Jul 1, 2024
7 of 8 checks passed
@bouweandela bouweandela deleted the 3699_pin_importlib_metadata branch July 1, 2024 17:24
@ehogan
Copy link
Contributor Author

ehogan commented Jul 2, 2024

Is this needed for the main branch? I understood that importlib_metadata version 8 is incompatible with esmpy < 8.6, but in the main branch esmpy is pinned to >= 8.6 since #3643.

Ah, yes, since importlib_metadata will be removed from ESMValCore after this release (see my comment on #3699), it makes sense to add this change to the release branch. I have updated the PR to target the release branch 👍

Although, I just realised that we'll need to merge the ESMValTool release branch back into main, so this change will end up on main anyway, unless I revert the change on the release branch after it is tagged but before the merge to main? 🤔

@bouweandela
Copy link
Member

Did you not tag a release candidate before creating the release branch?

@ehogan
Copy link
Contributor Author

ehogan commented Jul 2, 2024

Did you not tag a release candidate before creating the release branch?

Not for ESMValTool, because I thought a release candidate wasn't created for ESMValTool? (There aren't any release candidates listed at https://github.com/ESMValGroup/ESMValTool/releases.)

@bouweandela
Copy link
Member

Not previously, but it would be nice to start doing that to avoid the merge the release branch back into the main branch issue. Let's do it next release then and merge back for now.

@ehogan
Copy link
Contributor Author

ehogan commented Jul 2, 2024

It is possible to do it for this release; I would tag main before the esmpy PR I reverted, recreate the release branch from that tag, then cherry-pick everything after the esmpy PR (14 PRs) onto the release branch. Happy to do that if you'd prefer? 😊

@bouweandela
Copy link
Member

Yes, that would be great, if it's not too much work, you seem busy enough as it is.

@bouweandela
Copy link
Member

I think you could also tag the current main branch, create the release branch, and cherry pick c7abf57 and c0ab625 into it.

@ehogan
Copy link
Contributor Author

ehogan commented Jul 2, 2024

I think you could also tag the current main branch, create the release branch, and cherry pick c7abf57 and c0ab625 into it.

Ok, that sounds good, I'll try that 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Pin importlib_metadata <8
3 participants