-
-
Notifications
You must be signed in to change notification settings - Fork 7.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
feat(cli): use project name from package json by default #20479
base: develop
Are you sure you want to change the base?
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
0cdc9f0
to
0c96b74
Compare
beda654
to
b731004
Compare
b731004
to
360d43a
Compare
360d43a
to
ab08c32
Compare
ab08c32
to
0b891c4
Compare
0b891c4
to
6a88b01
Compare
defaultValues.name = newDefaultName; | ||
const nameQuestion = questions.find((q) => q.name === 'name'); | ||
if (nameQuestion) { | ||
nameQuestion.default = newDefaultName; | ||
} |
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.
Can you avoid mutating parameters, please?
I believe we want to go towards functional programming and pure functions. 👍
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.
Nice thought, function updated!
472ddf5
to
017e2a8
Compare
if (question.name === 'name') { | ||
return { ...question, default: newDefaultName }; | ||
} | ||
return { ...question }; |
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.
Do we really need to create a fresh object?
return { ...question }; | |
return question; |
If the point was to copy the object to not keep the reference, then you might want to do a deepClone instead of destructuring. 🤔
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.
Went for the JSON parse/stringify to do the copy, although it'll only work with simple objects/array (The initial code would've at least kept the functions if they were added to the question)
The only solution without any "effective" drawback would be to add a proper class or factory to instantiate the "question" and "default values" objects and use that instead. You think it's worth refactoring now?
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 you didn't responded to my first question 😄
But in any case, if we really want to copy, we can use https://lodash.com/docs/4.17.15#cloneDeep
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.
Indeed, I want to avoid having the same references. Initial code works for the current code since we only have scalar values so far.
Using cloneDeep now, should be better!
017e2a8
to
78eda62
Compare
78eda62
to
c5e0a87
Compare
c5e0a87
to
56fc5d8
Compare
56fc5d8
to
b796761
Compare
b796761
to
992d8a5
Compare
992d8a5
to
1b41d1a
Compare
What does it do?
When prompting the user for the name of the project during the deployment process, the setDefaultName function sets the default project name to the name of the existing Strapi project (retrieved from package.json), instead of using a generic default name like "my-strapi-project".
To run:
yarn test:unit
from the cloud folder.Why is it needed?
This change is needed to improve the user experience during the deployment process
How to test it?
yarn build
in the strapi/strapi folder containing this code's repositorystrapi.js
command to run thelogin
commanddeploy
command:When asked for the name of the project, check that the default value is the name of your existing project (as specified in package.json), not "my-strapi-project".
Result:
Related issue(s)/PR(s)
Let us know if this is related to any issue/pull request