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

Miscellaneous improvements to our documentation and help messages #467

Merged
merged 5 commits into from
Sep 26, 2023

Conversation

godlygeek
Copy link
Contributor

Before adding a description= to our ArgumentParser's:

$ memray parse --help
usage: memray parse [-h] results

positional arguments:
  results     Results of the tracker run

options:
  -h, --help  show this help message and exit

Please submit feedback, ideas, and bug reports by filing a new issue at https://github.com/bloomberg/memray/issues

After:

$ memray parse --help
usage: memray parse [-h] results

Debug a results file by parsing and printing each record in it

positional arguments:
  results     Results of the tracker run

options:
  -h, --help  show this help message and exit

Please submit feedback, ideas, and bug reports by filing a new issue at https://github.com/bloomberg/memray/issues

Before setting COLUMNS in our docs/conf.py:
image

After:
image

Match the spelling used by the Python documentation.

Signed-off-by: Matt Wozniski <mwozniski@bloomberg.net>
Prior to this change, our one-sentence description of each Memray
command would show up in the `memray --help` output (next to the name of
each subcommand), but wouldn't show up in the `memray subcommand --help`
output (at the top of the output, above the usage text), nor in the
generated CLI documentation in our HTML pages. Including this looks
nicer and improves readability for the help text.

Signed-off-by: Matt Wozniski <mwozniski@bloomberg.net>
If we don't do this, apparently `sphinx-argparse` inherits the
`argparse` default of formatting the help messages as wide as the
terminal that the docs are being built in, which results in different
widths for the pre-formatted usage text in our HTML documentation
depending on who builds the docs and how they do it.

Signed-off-by: Matt Wozniski <mwozniski@bloomberg.net>
We tried to be clever and avoid unnecessary work by skipping the
building and testing of wheels when only documentation was changed, but
this doesn't play nicely with our GitHub branch protection rules, since
it means that we never get the successful status checks that we require.

We could probably do something more clever to make the status check
immediately succeed without doing any real work when only documentation
was changed, but for now let's just revert back to doing the extra work.

Signed-off-by: Matt Wozniski <mwozniski@bloomberg.net>
This reverts commit 66a88b5.

Apparently codecov-action v4 is still a beta release. After we upgraded
to it, the maintainers changed its tags to correctly flag it as beta,
and now GitHub Actions is refusing to use it. Let's downgrade back to v3
for now.

Signed-off-by: Matt Wozniski <mwozniski@bloomberg.net>
@godlygeek godlygeek self-assigned this Sep 26, 2023
@codecov-commenter
Copy link

Codecov Report

All modified lines are covered by tests ✅

Comparison is base (c016ce5) 91.97% compared to head (c7f00a7) 91.94%.
Report is 2 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #467      +/-   ##
==========================================
- Coverage   91.97%   91.94%   -0.03%     
==========================================
  Files          91       91              
  Lines       10803    10803              
  Branches     1485     1485              
==========================================
- Hits         9936     9933       -3     
- Misses        865      868       +3     
  Partials        2        2              
Flag Coverage Δ
cpp 85.15% <ø> (-0.09%) ⬇️
python_and_cython 95.41% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

Files Coverage Δ
src/memray/_ipython/flamegraph.py 95.45% <ø> (ø)
src/memray/commands/attach.py 59.17% <ø> (ø)
src/memray/commands/run.py 89.10% <ø> (ø)

... and 2 files with indirect coverage changes

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

# Limit the width of usage messages. argparse defaults to the terminal width,
# but we don't want different output depending on the terminal width where the
# docs were built.
os.environ["COLUMNS"] = "88"
Copy link
Contributor

Choose a reason for hiding this comment

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

Much better, thank you for this change it is indeed much clearer this way 👍🏼

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think the last push to the docs was done manually by me, in a fullscreen terminal on a widescreen monitor, and I think that's why the usage summaries are currently a one-line mess with a scrollbar. I don't think we would have gotten a scrollbar had we let CI do an automated publish, because I don't think it would have wound up with the idea that the screen is so wide.

But also to be fair, I had no idea my environment was bleeding into the generated docs like this, so yeah, it's definitely much wiser to hardcode a sane value instead.

@godlygeek godlygeek merged commit 98032ae into bloomberg:main Sep 26, 2023
35 of 36 checks passed
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.

None yet

3 participants