-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
change dataSource to datasource #10438
base: master
Are you sure you want to change the base?
change dataSource to datasource #10438
Conversation
Pull Request Test Coverage Report for Build 8738018459Details
💛 - Coveralls |
6a4d7fc
to
d2d223f
Compare
@dhmlau please have a look at my PR. |
6f19bce
to
347337b
Compare
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.
Looks good to me. It becomes consistent with other commands. rest-crud uses keyword datasource
.
Hello @samarpanB. Can you please have a look at this PR? |
docs/site/Discovering-models.md
Outdated
@@ -35,7 +35,7 @@ Models can be discovered from a supported datasource by running the | |||
|
|||
### Options | |||
|
|||
`--dataSource`: Put a valid datasource name here to skip the datasource prompt | |||
`--datasource`: Put a valid datasource name here to skip the datasource prompt |
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.
Rather than replacing the old prompt, can we not support both old and new. This will ensure backward compatibility. Otherwise it will break existing cli command.
8f67a50
to
2fd6fc8
Compare
9ec6487
to
28c7c11
Compare
5e27fd4
to
42ced83
Compare
Signed-off-by: warisniz02 <[email protected]> Signed-off-by: warisniz02 <[email protected]> Signed-off-by: warisniz02 <[email protected]> Signed-off-by: warisniz02 <[email protected]> Signed-off-by: warisniz02 <[email protected]> Signed-off-by: warisniz02 <[email protected]> Signed-off-by: warisniz02 <[email protected]>
564fb5e
to
ade4914
Compare
@samarpanB please have a look at my PR! |
if ( | ||
this.dataSourceChoices | ||
.map(d => d.name) | ||
.includes(this.options.dataSource) | ||
.includes(this.options.dataSource || this.opetions.datasource) |
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.
.includes(this.options.dataSource || this.opetions.datasource) | |
.includes(this.options.dataSource || this.options.datasource) |
The
lb4 discoverer
names the argument to pass datasource as "dataSource". This PR makes the argument consistent with thelb4 datasource
without breaking existing cli command 'lb4 dataSource'.Checklist
npm test
passes on your machinepackages/cli
were updatedexamples/*
were updated👉 Check out how to submit a PR 👈