Skip to content

Conversation

@tweska
Copy link
Member

@tweska tweska commented Oct 24, 2025

I realized the example in our new formatting script does not correctly pick up the right files, then ended up tweaking a bunch of stuff:

  • Check the extension for c/c++ specific formatting
  • Add white space related fixes
  • Update pre-commit hook to run the formatting script (but not actually make changes)
  • Mention how to use pre-commit hook in the readme
  • Remove // at the end of lines in the mars2grib rules
  • Run the formatter

🌈🌦️📖🚧 Documentation 🚧📖🌦️🌈
https://sites.ecmwf.int/docs/dev-section/multio/pull-requests/PR-188

@codecov-commenter
Copy link

codecov-commenter commented Oct 24, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 56.61%. Comparing base (4999369) to head (bd2df7b).

Additional details and impacted files
@@           Coverage Diff            @@
##           develop     #188   +/-   ##
========================================
  Coverage    56.61%   56.61%           
========================================
  Files          316      316           
  Lines        20304    20304           
  Branches      1569     1569           
========================================
  Hits         11496    11496           
  Misses        8808     8808           

☔ 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.

@tweska tweska requested review from dsarmany and pgeier October 24, 2025 17:57
@pgeier
Copy link
Contributor

pgeier commented Oct 27, 2025

We have quite a few opened PR, maybe we wait with formatting for some more to be closed??

In the mars2grib/Rules.cc the trailing // have been introduced to customize formatting and make the rules more readable. Have you tried running formating it multiple times? The format on these might change on more time and become quite clumpy.

Alternative was to disable clang-format completely through comments // clang-format off. The advantage of // is that is still formats.

I would prefer not to touch the Rules.cc for now, there are more changes coming.

@tweska
Copy link
Member Author

tweska commented Oct 27, 2025

We have quite a few opened PR, maybe we wait with formatting for some more to be closed??

Yes, this is also what I suggested in the team meeting today. I will remove the commits that actually reformat code.

Alternative was to disable clang-format completely through comments // clang-format off. The advantage of // is that is still formats.

This is what I have already done in this PR, but I will remove that commit so that you can put changes on top without conflicts. Then we can always apply the changes manually again, it takes only a few minutes with find and replace.

Fix usage example

Remove trailing whitespaces

Remove trailing newlines

Consider file extension during formatting
Run the formatting script and check for changes
Mention command to enable git hooks

Add some extra whitespace
@tweska tweska force-pushed the feature/formatting branch from e33e9fc to bd2df7b Compare October 27, 2025 14:13
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.

4 participants