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

CLIENT-SPECIFICATION: Require support for long options #9651

Merged

Conversation

pixelcmtd
Copy link
Member

Inspired by: #9252 (comment)
For a project that emphasizes long and readable options as much as we do (imho often to an unnecessary degree), it is honestly completely incomprehensible to me that we would only support short options, and I have yet to come across any reasonable excuse for that. If you have one, please comment.

@pixelcmtd pixelcmtd added documentation Issues/PRs modifying the documentation. decision A (possibly breaking) decision regarding tldr-pages content, structure, infrastructure, etc. labels Dec 21, 2022
@EmilyGraceSeville7cf
Copy link
Contributor

No mandatory fixes found, approved.
Thanks for a contribution! 😆 🚀 ❤️

@waldyrious
Copy link
Member

waldyrious commented Dec 22, 2022

Thanks for the proposal, @pixelcmtd — indeed it makes sense!

[tldr-pages] emphasizes long and readable options [...] imho often to an unnecessary degree

I hear you, and I agree that it's sometimes excessive/unjustified. I have ideas for changes to the guidelines that would make them clearer and more sensible about when to use long vs. short options.

Copy link
Member

@waldyrious waldyrious left a comment

Choose a reason for hiding this comment

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

I agree with this change, but I'd ask other maintainers to wait a few days (perhaps over a week, given the holiday season) before merging, so that client authors may get the chance to have a look and chime in. Perhaps we should even ping authors of the main clients just to be sure. Does anyone have a list of client authors handy?

@pixelcmtd
Copy link
Member Author

@waldyrious I just looked through all clients listed in the wiki and the only somewhat spec compliant ones without long options are untldr and an old go implementation. The overwhelming majority of clients that respect the spec support long options.

@kbdharun
Copy link
Member

kbdharun commented Dec 24, 2022

@waldyrious I just looked through all clients listed in the wiki and the only somewhat spec compliant ones without long options are untldr and an old go implementation. The overwhelming majority of clients that respect the spec support long options.

Nice so I guess we can merge this PR. As suggested below we can wait for the client's author's opinions.

@pixelcmtd
Copy link
Member Author

@kbdharun I'd also suggest still waiting a bit for others to comment their concerns

@waldyrious
Copy link
Member

Thanks for doing that review! The authors of those clients don't seem to be very active, but just in case, let's ping them here in case they'd want to update accordingly: @unInstance, @k3mist.

@sbrl
Copy link
Member

sbrl commented Dec 26, 2022

Yeah, this looks sensible. I think we didn't require the implementation of long options vs short options before 'cause of some arg parsing libs that don't support long options, but I don't see a reason why we shouldn't require long options now.

Also, @pixelcmtd can you update the changelog at the bottom of the client spec before this is merged?

imho often to an unnecessary degree

agreed

@kbdharun kbdharun changed the title CLIENT-SPECIFICATION: Require long options CLIENT-SPECIFICATION: Require long options; add tasks to unreleased Dec 27, 2022
CLIENT-SPECIFICATION.md Outdated Show resolved Hide resolved
@CLAassistant
Copy link

CLAassistant commented Dec 28, 2022

CLA assistant check
All committers have signed the CLA.

Co-authored-by: Waldir Pimenta <waldyrious@gmail.com>
@pixelcmtd pixelcmtd changed the title CLIENT-SPECIFICATION: Require long options; add tasks to unreleased CLIENT-SPECIFICATION: Require support for long options Dec 29, 2022
Copy link
Contributor

@EmilyGraceSeville7cf EmilyGraceSeville7cf left a comment

Choose a reason for hiding this comment

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

No fixes found, approved.
Thanks for a contribution! 🚀 ❤️

@EmilyGraceSeville7cf EmilyGraceSeville7cf merged commit cd8ebad into tldr-pages:main Dec 29, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
decision A (possibly breaking) decision regarding tldr-pages content, structure, infrastructure, etc. documentation Issues/PRs modifying the documentation.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants