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

minor man page improvements #6519

Merged
merged 3 commits into from
Dec 17, 2024
Merged

Conversation

garlick
Copy link
Member

@garlick garlick commented Dec 17, 2024

Here are a few fixes to man page deficiencies that I noticed.

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, just those commit message nits i found

@@ -440,6 +440,13 @@ plugins include:
takes place on all the job's nodes if the scope is local, versus only the
first node of the job if the scope is global.

.. warning::
Copy link
Member

Choose a reason for hiding this comment

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

commit message s/beter/better

Copy link
Member

Choose a reason for hiding this comment

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

commit message s/drived/drifted/ ?

Copy link
Member Author

Choose a reason for hiding this comment

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

Oof, good catch!

@garlick
Copy link
Member Author

garlick commented Dec 17, 2024

Huh, this unit test failed on el8. I'll run it again but it's an odd one to fail.

2024-12-17T19:24:13.6630037Z not ok 261 - 5 file descriptors are open (expected 3-4)
2024-12-17T19:24:13.6630532Z FAIL: test_subprocess.t 261 - 5 file descriptors are open (expected 3-4)
2024-12-17T19:24:13.6631112Z #   Failed test '5 file descriptors are open (expected 3-4)'
2024-12-17T19:24:13.6631414Z #   at test/subprocess.c line 1132.

Copy link
Contributor

@grondo grondo left a comment

Choose a reason for hiding this comment

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

LGTM! Great changes here!

of higher level commands such as :man1:`flux-submit`.
must also be set for the stream.

See also: :option:`flux-submit --output`, :option:`flux-submit --error`.
Copy link
Contributor

Choose a reason for hiding this comment

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

Oh, this was smart!

@garlick
Copy link
Member Author

garlick commented Dec 17, 2024

Thanks guys - I'll set MWP

Problem: a warning box intended to clarify a point about stage-in
file paths has drifted away from the shell stage-in options.

Move warning box back to its original location.
Problem: users may be unaware of the nicer flux-submit shorthands
for shell options when perusing flux-shell(1).

Add some explicit linked references and normalize the form of the
references that already exist.
Problem: the environment available to prolog, epilog, and housekeeping
scripts is not documented in the environment man page.

Add a section.
@mergify mergify bot merged commit 368b85a into flux-framework:master Dec 17, 2024
35 checks passed
Copy link

codecov bot commented Dec 17, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 83.61%. Comparing base (a9666a8) to head (95ed4fd).
Report is 4 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #6519      +/-   ##
==========================================
- Coverage   83.66%   83.61%   -0.06%     
==========================================
  Files         522      522              
  Lines       87734    87734              
==========================================
- Hits        73401    73356      -45     
- Misses      14333    14378      +45     

see 12 files with indirect coverage changes

@garlick garlick deleted the doc_improvements branch December 17, 2024 23:30
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.

3 participants