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

flux-job: do not override -L, --color when -H, --human is used in eventlog and wait-event subcommands #6612

Merged
merged 2 commits into from
Feb 6, 2025

Conversation

grondo
Copy link
Contributor

@grondo grondo commented Feb 6, 2025

While working on something else I discovered that flux job eventlog -HL ... wasn't forcing color on like it should. This is because -H, --human forces color = auto, overwriting the user's command line choice.

auto is already the default. Stop doing that.

Add a test.

Problem: The `-H, --human` option of `flux job eventlog` and
`flux job wait-event` forces `--color=auto`, but this breaks usage
like `-HL` to force color on (e.g. if passed to `less`).

Do not overwrite the color option with `--human`. The default is
already `auto`, so there's no need.
Problem: The `flux job wait-event` and `eventlog` tests do not ensure
that the options `-HL` enable human timestamps but also force color.

Add this option usage to existing tests that check that color is
force-enabled.
Copy link
Member

@chu11 chu11 left a comment

Choose a reason for hiding this comment

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

LGTM!

@grondo
Copy link
Contributor Author

grondo commented Feb 6, 2025

Thanks! Setting MWP

@mergify mergify bot merged commit 34a392f into flux-framework:master Feb 6, 2025
35 checks passed
Copy link

codecov bot commented Feb 6, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 79.51%. Comparing base (496a332) to head (3a175c2).
Report is 3 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #6612      +/-   ##
==========================================
- Coverage   79.52%   79.51%   -0.01%     
==========================================
  Files         531      531              
  Lines       88354    88353       -1     
==========================================
- Hits        70264    70257       -7     
- Misses      18090    18096       +6     
Files with missing lines Coverage Δ
src/cmd/job/eventlog.c 89.36% <ø> (-0.06%) ⬇️

... and 8 files with indirect coverage changes

@grondo grondo deleted the color-eventlog branch February 6, 2025 23:43
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.

2 participants