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

Remove unnecessary conditional from print action #48

Merged
merged 1 commit into from
Jan 15, 2025

Conversation

stefaniereuter
Copy link
Contributor

No change in behavior just deleting an unnecessary conditional. msg.tag()==message::Message::Tag::Field is already either true or false and does not need to be matched with identical bools in a conditional.

@MircoValentiniECMWF
Copy link
Collaborator

MircoValentiniECMWF commented Jan 15, 2025

It will be useful to have the "only-flush" option in the print action, for a fast first attempt to debug flush related problems.

@codecov-commenter
Copy link

codecov-commenter commented Jan 15, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 22.91%. Comparing base (68d90cc) to head (5b6b262).

Additional details and impacted files
@@           Coverage Diff            @@
##           develop      #48   +/-   ##
========================================
  Coverage    22.91%   22.91%           
========================================
  Files          292      292           
  Lines        30589    30589           
  Branches      1169     1169           
========================================
  Hits          7009     7009           
  Misses       23580    23580           

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

Copy link
Collaborator

@dsarmany dsarmany left a comment

Choose a reason for hiding this comment

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

Looks fine to me.

I also like the commit message, but please always capitalise it, so 'remove...' -> 'Remove...'

@stefaniereuter stefaniereuter changed the title remove unnecessary conditional from print action Remove unnecessary conditional from print action Jan 15, 2025
@dsarmany
Copy link
Collaborator

It will be useful to have the "only-flush" option in the print action, for a fast first attempt to debug flush related problems.

Do you want this to be in this PR? I think it may make sense to have it as a separate, small feature request with the small test. that demonstrates its use.

@stefaniereuter
Copy link
Contributor Author

Yes I agree should be in a separate feature branch.

@stefaniereuter
Copy link
Contributor Author

I've changed the commit message

@dsarmany dsarmany force-pushed the feature/cleanup-print branch from 57758cf to 5b6b262 Compare January 15, 2025 15:18
@dsarmany dsarmany merged commit a764fc4 into develop Jan 15, 2025
128 checks passed
@dsarmany dsarmany deleted the feature/cleanup-print branch January 15, 2025 16:31
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