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

Removed deprecated configuration option offline #2213

Merged
merged 3 commits into from
Oct 4, 2023
Merged

Conversation

schlunma
Copy link
Contributor

@schlunma schlunma commented Oct 2, 2023

Description

This PR removes the deprecated configuration option offline.

Detailed upgrade instructions: #1935

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:

@schlunma schlunma added this to the v2.10.0 milestone Oct 2, 2023
@schlunma schlunma self-assigned this Oct 2, 2023
@codecov
Copy link

codecov bot commented Oct 2, 2023

Codecov Report

Merging #2213 (65b2bd1) into main (837ce35) will decrease coverage by 0.03%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##             main    #2213      +/-   ##
==========================================
- Coverage   93.21%   93.19%   -0.03%     
==========================================
  Files         238      238              
  Lines       12851    12840      -11     
==========================================
- Hits        11979    11966      -13     
- Misses        872      874       +2     
Files Coverage Δ
esmvalcore/_main.py 90.94% <ø> (+0.33%) ⬆️
esmvalcore/config/_config_validators.py 97.29% <100.00%> (-0.16%) ⬇️

... and 1 file with indirect coverage changes

@schlunma schlunma marked this pull request as ready for review October 2, 2023 08:56
Copy link
Member

@bouweandela bouweandela left a comment

Choose a reason for hiding this comment

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

There is a bit of code left in esmvalcore/_main.py that is also part of this feature, could you have a look?
https://github.com/ESMValGroup/ESMValCore/blob/remove_offline/esmvalcore/_main.py#L368-L376

@schlunma
Copy link
Contributor Author

schlunma commented Oct 2, 2023

Whoopsie, looks like I messed up the git add somehow. Should be fine now.

@bouweandela
Copy link
Member

It looks like the feature is also used in the backward compatibility tests: https://github.com/ESMValGroup/ESMValTool/blob/main/.circleci/config.yml#L217

@schlunma
Copy link
Contributor Author

schlunma commented Oct 2, 2023

Good catch, see ESMValGroup/ESMValTool#3367.

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.

gut work! Also approved Manu's Tool PR that removes it from there

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.

3 participants