Skip to content

fix: mars2grib guard HAVE_ECKIT_GEO#210

Merged
danovaro merged 1 commit into
developfrom
fix/guard-mars2grib-eckitgeo
May 8, 2026
Merged

fix: mars2grib guard HAVE_ECKIT_GEO#210
danovaro merged 1 commit into
developfrom
fix/guard-mars2grib-eckitgeo

Conversation

@mcakircali
Copy link
Copy Markdown
Contributor

Description

mars2grib_libs must be guarded with HAVE_ECKIT_GEO

Contributor Declaration

By opening this pull request, I affirm the following:

  • All authors agree to the Contributor License Agreement.
  • The code follows the project's coding standards.
  • I have performed self-review and added comments where needed.
  • I have added or updated tests to verify that my changes are effective and functional.
  • I have run all existing tests and confirmed they pass.

@mcakircali mcakircali requested a review from danovaro May 8, 2026 10:16
Copy link
Copy Markdown
Member

@tweska tweska left a comment

Choose a reason for hiding this comment

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

eckit::geo is not an optional dependency for the mars2grib encoder. Correct me if I am wrong, but I think this change makes it "optional" and then it will just crash the build.

I think instead we must set the condition HAVE_ECKIT_GEO here:

metkit/CMakeLists.txt

Lines 65 to 68 in cbfcd75

ecbuild_add_option( FEATURE MARS2GRIB
DEFAULT ON
CONDITION HAVE_GRIB
DESCRIPTION "Build the MARS2GRIB encoder" )

@MircoValentiniECMWF
Copy link
Copy Markdown
Contributor

MircoValentiniECMWF commented May 8, 2026

I fully agree with Kevin

@mcakircali mcakircali force-pushed the fix/guard-mars2grib-eckitgeo branch from 0b14ad2 to 7ff20fe Compare May 8, 2026 11:23
@mcakircali mcakircali requested a review from tweska May 8, 2026 11:32
Copy link
Copy Markdown
Member

@danovaro danovaro left a comment

Choose a reason for hiding this comment

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

thanks @mcakircali

@danovaro danovaro merged commit 2384341 into develop May 8, 2026
116 of 133 checks passed
@danovaro danovaro deleted the fix/guard-mars2grib-eckitgeo branch May 8, 2026 11:33
@tweska
Copy link
Copy Markdown
Member

tweska commented May 8, 2026

Thanks for the fix @mcakircali!

@codecov-commenter
Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 73.36%. Comparing base (cbfcd75) to head (7ff20fe).
⚠️ Report is 2 commits behind head on develop.

Additional details and impacted files
@@             Coverage Diff              @@
##           develop     #210       +/-   ##
============================================
+ Coverage    62.02%   73.36%   +11.33%     
============================================
  Files          303      126      -177     
  Lines        11672     8137     -3535     
  Branches      1049      778      -271     
============================================
- Hits          7240     5970     -1270     
+ Misses        4432     2167     -2265     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants