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 fixes/improvements to docs #144

Merged
merged 1 commit into from
Nov 28, 2022
Merged

Conversation

alexreg
Copy link
Contributor

@alexreg alexreg commented Nov 28, 2022

The second commit is just the product of running black & isort on the codebase, which I thought might be nice, but obviously feel free to take that or leave it! I think most of the little edits in the first commit are uncontroversial. :-)

@janluke
Copy link
Owner

janluke commented Nov 28, 2022

Oh, I didn't know that Python convention on the grammatical person to use in docstrings was the opposite of the Java one.

The docstring is a phrase ending in a period. It prescribes the function or method’s effect as a command (“Do this”, “Return that”), not as a description; e.g. don’t write “Returns the pathname …”.

I have to admit that Java one makes more sense to me but conventions are conventions. Thank you so much for all these refinements!

Regarding introducing black and isort, that deserves a PR on its own, it cannot be introduced so lightly since it changes a lot of lines and the chance of causing conflicts with my feature branches is pretty high. Besides, while isort is uncontroversial, I tend to avoid black if I can. I've been the only one developing cloup, so there was not much point in using it: very often it uglifies the code and make it take too much vertical space for my taste.

So, I'd accept a PR for using isort pretty easily (possibly with pre-commit integration). But for black, I need to think about it.

@alexreg
Copy link
Contributor Author

alexreg commented Nov 28, 2022

Yes, you're right, I'm not sure why I tacked the black/isort stuff onto this PR! It was sort of an afterthought, I suppose. Mainly to make spacing around docstrings more consistent. (And whether to start them on the same line as the """.)

I meant to explain about the verb form used for functions/methods, but glad you found that anyway... indeed, it surprised me too at first, since C# is like Java in its convention.

I'll just knock off the second commit here, and let you add isort if you fancy it. (Very easily done, of course.)

Anyway, thanks for your continued good work on Cloup. It was a pretty pleasant experience to fork typer and convert it to use Cloup instead of Click. (I've made a few releases of typer-cloup now.)

@alexreg
Copy link
Contributor Author

alexreg commented Nov 28, 2022

(Just fixing something in the first commit now; will push again soon.)

All done!

@alexreg alexreg changed the title Miscellaneous fixes/improvements to docs & reformatting Miscellaneous fixes/improvements to docs Nov 28, 2022
@janluke janluke merged commit 23d32c4 into janluke:master Nov 28, 2022
@alexreg alexreg deleted the clean-up branch November 28, 2022 22:43
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.

2 participants