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

Some docs housekeeping and minor cleanups #1752

Merged
merged 6 commits into from
Jun 15, 2023
Merged

Some docs housekeeping and minor cleanups #1752

merged 6 commits into from
Jun 15, 2023

Conversation

meatballhat
Copy link
Member

What type of PR is this?

  • cleanup
  • documentation

What this PR does / why we need it:

  • the .flake8 file is no longer used
  • I felt that the first section of the top-level README.md didn't help explain why someone might choose to use this project and the docs/index.md intro should probably be the same (opinions wanted)
  • the LICENSE year was outdated

Which issue(s) this PR fixes:

N/A 😬

@meatballhat meatballhat added kind/documentation describes a documentation update kind/cleanup describes internal cleanup / maintaince area/v3 relates to / is being considered for v3 labels Jun 12, 2023
@meatballhat meatballhat added this to the Release 3.x milestone Jun 12, 2023
@meatballhat meatballhat requested a review from a team as a code owner June 12, 2023 12:45
flag.go Outdated Show resolved Hide resolved
dearchap
dearchap previously approved these changes Jun 13, 2023
README.md Outdated Show resolved Hide resolved
urfave/cli is a **declarative**, simple, fast, and fun package for building command line tools in Go featuring:

- nestable commands with alias and prefix match support
- flexible and permissive help system
Copy link
Contributor

Choose a reason for hiding this comment

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

Permissive help? What does it permit?

Copy link
Member Author

Choose a reason for hiding this comment

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

I was thinking of how the default behavior allows help before and after positional commands, and has a default h alias, and also the --help -h flags, although these behave in a more standard way. I picked "permissive" because I wanted a single word that describes how the help system is trying to be compatible with various usage styles.

Copy link
Contributor

Choose a reason for hiding this comment

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

Then it is better to expand it to explain.

  • allow -h, --help before and after positional commands

Before means app help, and after means command help, right?

Copy link
Member Author

Choose a reason for hiding this comment

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

The ordering is true for the flag form, but the behavior is a little different for the command form which treats adjacent command name and help as equivalent no matter the order, meaning these are equivalent:

prog cmd help
prog help cmd

Co-authored-by: Anatoli Babenia <anatoli@rainforce.org>
README.md Outdated
- flexible and permissive help system
- dynamic shell completion for `bash`, `zsh`, `fish`, and `powershell`
- `man` and markdown format documentation generation
- input flag types for primitives, slices of primitives, time, duration, and others
Copy link
Contributor

Choose a reason for hiding this comment

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

The terminology "primitives" and "slices of primitives" seems unique here. Simple types is better. Why would it be a feature?

Copy link
Member Author

Choose a reason for hiding this comment

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

I like "simple types" 👍 How would you like to see it worded?

Copy link
Contributor

Choose a reason for hiding this comment

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

Depends on what purpose does this feature serve. Why I would need type autoconversion at all? It may bloat the library.

Copy link
Member Author

Choose a reason for hiding this comment

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

I don't follow 😅 Maybe it would help for me to define the audience more clearly (?) I'm attempting to speak to a software engineer who has worked with Go and knows about the standard library flag package.

Copy link
Contributor

Choose a reason for hiding this comment

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

https://pkg.go.dev/flag doesn't mention the word "primitive" so it may be unclear even for those who used flag.

Copy link
Member Author

Choose a reason for hiding this comment

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

@abitrolly I'm in favor of replacing "primitive" with "simple" 👍🏼 I wasn't following what you meant by:

Depends on what purpose does this feature serve. Why I would need type autoconversion at all? It may bloat the library.

Copy link
Contributor

Choose a reason for hiding this comment

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

@meatballhat my first language is Python, so the comment was that the library exposes too much types. Uint, Float64, which are too specific for a command line API. And if I understand it correctly, if these types are not used, they will still take space in importing app.

README.md Outdated Show resolved Hide resolved
Co-authored-by: Anatoli Babenia <anatoli@rainforce.org>
More documentation is available in [`./docs`](./docs) or the hosted
documentation site at <https://cli.urfave.org>.
More documentation is available in [`./docs`](./docs) or the hosted documentation site published from the latest release
at <https://cli.urfave.org>.
Copy link
Contributor

Choose a reason for hiding this comment

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

Copy link
Member Author

Choose a reason for hiding this comment

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

I'm not sure how best to handle the pkg.go.dev docs in relation to the ./docs tree that's published via mkdocs and the gh-pages branch. Do you think mentioning it here is good? More than the link button alone, that is?

Copy link
Contributor

Choose a reason for hiding this comment

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

I would skip that, but if there us a dedicated documentation section, the autogenerated godoc API should be mentioned there. There is no replacement for it in ./docs.

dearchap
dearchap previously approved these changes Jun 14, 2023
@meatballhat meatballhat merged commit b335c28 into main Jun 15, 2023
@meatballhat meatballhat deleted the housekeeping branch June 15, 2023 11:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/v3 relates to / is being considered for v3 kind/cleanup describes internal cleanup / maintaince kind/documentation describes a documentation update
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants