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

Add --version and auto-update README (enforced) #57

Merged
merged 8 commits into from
Jun 10, 2024

Conversation

asmacdo
Copy link
Member

@asmacdo asmacdo commented Jun 7, 2024

Fixes #48
Fixes #59

@asmacdo asmacdo marked this pull request as draft June 8, 2024 04:44
@asmacdo
Copy link
Member Author

asmacdo commented Jun 8, 2024

Marked as draft to include auto readme update #59

@asmacdo
Copy link
Member Author

asmacdo commented Jun 9, 2024

On last change needed-- run the script in gh actions and assert no diff.

@asmacdo asmacdo force-pushed the add-version branch 4 times, most recently from 8d212b3 to aa26bfc Compare June 10, 2024 00:11
@asmacdo
Copy link
Member Author

asmacdo commented Jun 10, 2024

The new CI check updates the README.md with the contents of duct --help

the diff

 -optional arguments:
+options:

in python 3.10 Argparse changed how they format helptext python/cpython#53903

option 1: we can either ignore that particular line, (but that will cause it to possibly change back and forth, so not ideal)

option 2: we lock python version in CI and in precommit to 3.10+, which is also not ideal

option 3 : we use python version in gh action check >= 3.10, but don't enforce python version in the precommit. We document that developers should undo that line if running precommit with python < 3.10

All 3 options are gross to me, but i lean option 3. @yarikoptic ideas, thoughts?

README.md Outdated

Gathers metrics on a command and all its child processes.

positional arguments:
command The command to execute.
arguments Arguments for the command. (default: None)
inner_args Arguments for the command.
Copy link
Member

Choose a reason for hiding this comment

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

in above usage: output there is now no inner_args, only ... . Ideally should be matched here

Copy link
Member Author

Choose a reason for hiding this comment

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

Updated in last push, hows this?

duct --help

usage: duct [-h] [--version] [-p OUTPUT_PREFIX] [--sample-interval SAMPLE_INTERVAL] [--report-interval REPORT_INTERVAL]
            [-c {all,none,stdout,stderr}] [-o {all,none,stdout,stderr}] [-t {all,system-summary,processes-samples}]
            command [inner_args ...] ...

Gathers metrics on a command and all its child processes.

positional arguments:
  command [inner_args ...]
                        The command to execute, along with its inner arguments.
  inner_args            Arguments for the command.

Copy link
Member

Choose a reason for hiding this comment

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

sorry for being pain, but inner_args is like a jargon which IMHO hard to communicate. Could you consistently rename it to command_args?

Copy link
Member Author

Choose a reason for hiding this comment

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

No pain, command_args is better, +1 Fixed in latest push

@yarikoptic
Copy link
Member

All 3 options are gross to me, but i lean option 3. @yarikoptic ideas, thoughts?

option 3 sounds good enough for me too!

@yarikoptic
Copy link
Member

Note that original PR intent/description (Add --version) is far from its current purpose/changes.

Co-authored-by: Yaroslav Halchenko <debian@onerussian.com>
@asmacdo asmacdo changed the title Add --version Add --version and auto-update README (enforced) Jun 10, 2024
@asmacdo asmacdo marked this pull request as ready for review June 10, 2024 14:00
@asmacdo asmacdo merged commit 0171257 into con:main Jun 10, 2024
22 checks passed
@asmacdo asmacdo deleted the add-version branch August 22, 2024 15:04
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.

automate update of readme with --help Add --version
2 participants