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.

Sign up for GitHub

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

Jump to bottom

Propagate verbose flag into PBS job script #293

Merged
merged 1 commit into from
May 9, 2024
Merged

Propagate verbose flag into PBS job script #293

merged 1 commit into from
May 9, 2024

Conversation

SeanBryan51
Copy link
Collaborator

@SeanBryan51 SeanBryan51 commented May 8, 2024 •
edited
Loading

Fixes #292

@SeanBryan51 SeanBryan51 linked an issue May 8, 2024 that may be closed by this pull request
Verbosity setting is not propagated into PBS job #292
Closed
@codecov Codecov
Copy link

codecov bot commented May 8, 2024 •
edited
Loading

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 70.07%. Comparing base (bb0ab3d) to head (61af6cb).
Report is 1 commits behind head on main.

Additional details and impacted files 
@@            Coverage Diff             @@
##             main     #293      +/-   ##
==========================================
+ Coverage   69.97%   70.07%   +0.09%     
==========================================
  Files          18       18              
  Lines         986      989       +3     
==========================================
+ Hits          690      693       +3     
  Misses        296      296

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

@SeanBryan51 SeanBryan51 requested review from a team and abhaasgoyal and removed request for a team May 8, 2024 06:07
@SeanBryan51 SeanBryan51 marked this pull request as ready for review May 8, 2024 06:07
bschroeter
bschroeter reviewed May 8, 2024
View reviewed changes
Copy link
Collaborator

@bschroeter bschroeter left a comment

Choose a reason for hiding this comment

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

Looks like it works, just suggesting a helper function if you have the time.

src/benchcab/benchcab.py Outdated Show resolved Hide resolved
bschroeter
bschroeter approved these changes May 8, 2024
View reviewed changes
Copy link
Collaborator

@bschroeter bschroeter left a comment

Choose a reason for hiding this comment

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

I'm going to go ahead and approve this - it would be nice to do a docstring for the new function (and type hinting for this and other utility functions) when time permits.

@SeanBryan51 SeanBryan51 force-pushed the 292-verbosity-setting-is-not-propagated-into-pbs-job branch from 67109b7 to 61af6cb Compare May 9, 2024 00:20
@SeanBryan51 SeanBryan51 merged commit fc625c0 into main May 9, 2024
4 checks passed
@SeanBryan51 SeanBryan51 deleted the 292-verbosity-setting-is-not-propagated-into-pbs-job branch May 9, 2024 01:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@bschroeter bschroeter bschroeter approved these changes

@abhaasgoyal abhaasgoyal Awaiting requested review from abhaasgoyal abhaasgoyal was automatically assigned from CABLE-LSM/tech-reviewers

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Verbosity setting is not propagated into PBS job
2 participants
@SeanBryan51 @bschroeter