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.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

fix: scriptName + improving usage messages #305

Merged
merged 2 commits into from Nov 29, 2021
Merged

fix: scriptName + improving usage messages #305

merged 2 commits into from Nov 29, 2021

Conversation

Lucas-C
Copy link
Contributor

@Lucas-C Lucas-C commented Jul 7, 2021

What: CLI usage was specified in the code, but in a wrong way.
This PR allows for the subcommands usage message to be displayed properly.
It also replace cli.js by all-contributors in the usage messages.

Why: in order to fix the aforementioned usage message issues

How: in Javascript 馃槉

Checklist:

  • Documentation N/A
  • Tests N/A
  • Ready to be merged
  • Added myself to contributors table

Behaviour BEFORE this PR:

$ all-contributors add --help

add a new contributor

Options :
...

Behaviour AFTER this PR:

$ all-contributors add --help

Add a new contributor

USAGE: all-contributors add <username> <comma-separated contributions>

Options :
...

My initial motivation for this PR was to make it explicit in all-contributors add --help
that several contributions could be specified at once,
but comma-separated and not space-separated,
as I have been confused myself by this.

Berkmann18
Berkmann18 reviewed Jul 13, 2021
View reviewed changes
other/MAINTAINING.md Outdated Show resolved Hide resolved
package.json Outdated Show resolved Hide resolved
@Lucas-C
Copy link
Contributor Author

Lucas-C commented Jul 13, 2021

I added a commit fixing the points raised in @Berkmann18 review

@Lucas-C Lucas-C changed the title Improving usage messages + fixing scriptName fix: scriptName + improving usage messages Jul 13, 2021
@Lucas-C
Copy link
Contributor Author

Lucas-C commented Jul 13, 2021

I could also add a warning in the addContribution function when extra CLI arguments are provided, in order to tell users that they are ignored and that a comma-separated list is expected instead.

@Berkmann18
Copy link
Member

I could also add a warning in the addContribution function when extra CLI arguments are provided, in order to tell users that they are ignored and that a comma-separated list is expected instead.

I wonder if having that in the help/usage section would be better, what do you think?

@Lucas-C
Copy link
Contributor Author

Lucas-C commented Jul 25, 2021

@Berkmann18 it's already in the usage section 馃槈
From https://github.com/all-contributors/all-contributors-cli/pull/305/files#diff-a3751a8ca1ab5bdf5d4bb6b4c3861d9a81205bc01b080e57301090b6e8b34165R26 :

.command('add', `Add a new contributor\n\nUSAGE: all-contributors add <username> <comma-separated contributions>`)

@Berkmann18
Copy link
Member

True, feel free to make a PR for that 馃榿.

@Lucas-C
Copy link
Contributor Author

Lucas-C commented Jul 25, 2021

@Berkmann18 You mean a separate one? Do you want this piece of code to be removed from this PR?
Sorry, I don't quite understand... 馃

@Lucas-C
Copy link
Contributor Author

Lucas-C commented Sep 1, 2021

Could this PR be merged?

@Berkmann18 : are you expecting anything on my part?

Berkmann18
Berkmann18 approved these changes Nov 29, 2021
View reviewed changes
Copy link
Member

@Berkmann18 Berkmann18 left a comment

Choose a reason for hiding this comment

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

Apologies for taking long to come back to this.

@Berkmann18 Berkmann18 merged commit d3b8f88 into all-contributors:master Nov 29, 2021
@Berkmann18
Copy link
Member

@all-contributors please add @Lucas-C for docs

@allcontributors allcontributors bot mentioned this pull request Nov 29, 2021
Merged
@allcontributors
Copy link
Contributor

@Berkmann18

I've put up a pull request to add @Lucas-C! 馃帀

@all-contributors-release-bot
Copy link
Member

馃帀 This PR is included in version 6.20.1 馃帀

The release is available on:

Your semantic-release bot 馃摝馃殌

tenshiAMD added a commit that referenced this pull request Sep 13, 2022
@tenshiAMD
Merge remote-tracking branch 'origin/master' into feat/style-table
6744c99 
* origin/master: (85 commits)
  refactor: log full error stack on error (#316)
  chore: fix status badges (#315)
  docs: add JoshuaKGoldberg as a contributor for bug (#314)
  fix: incorrect usage of `tbody` (#311)
  fix: trim `nextLink` before slicing (#309)
  fix: set default value as `7` for `contributorsPerLine` (#139)
  chore(deps): bump dependencies and devDeps (#298)
  refactor: add tbody to contributors table (#307)
  docs: add Lucas-C as a contributor for doc (#306)
  fix: scriptName + improving usage messages (#305)
  docs: add vapurrmaid as a contributor (#304)
  chore(deps): CVE-2021-23337 in inquirer->lodash (#303)
  docs: add SirWindfield as a contributor (#297)
  feat: add namespaced token (#296)
  docs: add LaChapeliere as a contributor (#292)
  feat(contribution-types): add research contribution type (#291)
  docs: add darekkay as a contributor (#290)
  feat: display a meaningful error when the config file is missing (#288)
  docs: add melink14 as a contributor (#285)
  docs: add jdalrymple as a contributor (#264)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Berkmann18 Berkmann18 Berkmann18 approved these changes

Assignees
No one assigned
Labels
released
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@Lucas-C @Berkmann18 @all-contributors-release-bot