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

Added yargs to list of popular alternatives in /docs #16

Merged
merged 2 commits into from
Oct 20, 2024

Conversation

dvidsilva
Copy link

*Issue number of the reported bug or feature request: #15

Describe your changes
Added a short paragraph about yargs to the popular alternatives

Testing performed
it compiles locally

Additional context
congrats on the launch

@dvidsilva dvidsilva requested a review from a team as a code owner October 11, 2024 18:24
Copy link
Member

@molisani molisani left a comment

Choose a reason for hiding this comment

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

Thanks for the contribution! Make sure you sign off your commits so that the DCO check is happy.

We did mention Yargs in the Common Patterns section under Method Chaining, but I agree that yargs is ubiquitous enough that it could be repeated under Popular Libraries.

That being said, I would want any description of the library to mention that its approach to TypeScript support requires types to be defined in the parser and then consumed by the implementation (rather than the other way around, which is the approach that Stricli takes).

@dvidsilva
Copy link
Author

makes total sense, thanks - making a change to include that.

also I just realized that it is installed in node_modules, is a dependency of some of the dependencies of the package.

@dvidsilva dvidsilva requested a review from molisani October 15, 2024 00:20
@mkubilayk
Copy link
Collaborator

@dvidsilva Can you sign off your last commit as well please?

https://github.com/bloomberg/stricli/pull/16/checks?check_run_id=31527772548 contains instructions.

@dvidsilva
Copy link
Author

@mkubilayk thanks! just did and force pushed.

@molisani
Copy link
Member

I've rebased the changes but it looks like the DCO check is still not passing.

Expected "Daveed daveed@santocabron.com", but got "David Silva cali@hey.com".

It looks like there's just some mismatch between your local git config and your GitHub profile.

@dvidsilva dvidsilva force-pushed the docs/yargs branch 3 times, most recently from 80b0f3d to 35b3f14 Compare October 20, 2024 00:52
@dvidsilva
Copy link
Author

right, I rebased and squashed them, one of them had a different email

thanks

Daveed and others added 2 commits October 20, 2024 12:20
Signed-off-by: Daveed <daveed@santocabron.com>
Signed-off-by: Michael Molisani <git@molisani.us>
@molisani molisani merged commit 2bb4b8d into bloomberg:main Oct 20, 2024
2 checks passed
@molisani
Copy link
Member

Sorry for some of the back and forth, but thank you very much for the contribution!

@molisani molisani added the documentation 📝 Improvements or additions to documentation label Oct 20, 2024
@dvidsilva
Copy link
Author

Appreciated! it was a great exercise, will send more complex PRs if I run into issues locally

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation 📝 Improvements or additions to documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants