-
Notifications
You must be signed in to change notification settings - Fork 2.7k
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
Use an alias system instead of camelCase function #3101
Conversation
- tests properly camelised comand - tests properly command with hyphen - move generation of documentation in src/cli/commands/index.js - move every command related stuff in src/cli/commands/index.js - tests some corner cases - delete every camelCase use in src/cli/commands/index.js and src/cli/commands/help.js
@arcanis I rebased after the last commits ;) |
src/cli/commands/index.js
Outdated
import * as version from './version.js'; commands['version'] = version; | ||
import * as versions from './versions.js'; commands['versions'] = versions; | ||
import * as why from './why.js'; commands['why'] = why; | ||
import * as upgradeInteractive from './upgrade-interactive.js'; commands['upgradeInteractive'] = upgradeInteractive; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It might be clearer to do something like this:
import * as bar from './bar';
import * as foo from './foo';
...
let commands = {
bar,
foo,
...
};
What do you think?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think is A LOT clearer; i will change it!
__tests__/index.js
Outdated
await expectAnInfoMessageAfterError( | ||
execCommand('i', [], 'run-add', true), | ||
'Visit https://yarnpkg.com/en/docs/cli/install for documentation about this command.', | ||
); | ||
}); | ||
|
||
test.concurrent('should show help and ignore unsopported aliases', async () => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
unsopported
? :p
(same typo on 214 and 221 :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
aaaah thanks and sorry XD
Nice work! Just a typo to fix and I merge this 👍 I really feel like github should offer a way to edit PRs from the review interface, for cases like this ... |
@arcanis done! |
Summary
In this pull request I try to remove the camelCase function from src/cli/commands/index.js and
src/cli/commands/help.js and instead I use an alias system.
I move the existing aliases to unsupported-aliases.
We can't still remove the entire dependency
camelCase
because global command use it.Finally I move every command-related stuff in src/cli/commands/index.js.
Test plan