fix: Fixing issues with commands introduced in #403#440
fix: Fixing issues with commands introduced in #403#440jdalrymple wants to merge 2 commits intoall-contributors:mainfrom
Conversation
Also replacing chalk with something lighter weight
| const contributions = argv._[2] | ||
|
|
||
| // Add or update contributor in the config file | ||
| const data = updateContributors(argv, username, contributions) |
There was a problem hiding this comment.
Also another thing i missed in that original PR. I forgot the await here.
There was a problem hiding this comment.
Ahh perfect. Justin, we can merge this FIRST, and then I'll rebase my tests pr against it.
There was a problem hiding this comment.
Thats what i was thinking!
| const argv = getArgs() | ||
|
|
||
| // Load and merge config file data into argv | ||
| try { |
There was a problem hiding this comment.
tldr; Original PR removed the .config prop of the yargs method because the docs mentioned it didnt support async functions.
What i forgot to do apparently, it replace it properly, so the config wasnt loaded like it should have been. This change fixes that correctly by loading the config on execution of the cli command.
| chalk.bold('Missing contributors in .all-contributorsrc:\n'), | ||
| YoctoColors.bold('Missing contributors in .all-contributorsrc:\n'), | ||
| ) | ||
| process.stdout.write(` ${missingInConfig.join(', ')}\n`) |
There was a problem hiding this comment.
We were indenting missing members, but not unknown members. It looked weird, so i removed the indent
lwasser
left a comment
lwasser
left a comment
There was a problem hiding this comment.
@jdalrymple, everything here looks good to me, and it runs locally 🎉 🎉 You also simplified the configData /. argv load complexity, which works well, and the config params load locally as expected!
Because we are introducing a different dependency, I just want @JoshuaKGoldberg to have a look, but the CLI runs great with this pr.
|
@all-contributors please add @jdalrymple code, bug |
|
@jdalrymple already contributed before to code, bug |
2 participants
What: A few issues with the commands were introduced by #403 whereby the config was not loaded where it should have been on the execution of the cli commands.
In addition to fixing this problem, I also:
Why: Poor testing on my end
How:
Checklist: