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.

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

ignore .idea folder #522

Merged
merged 2 commits into from
Aug 31, 2016
Merged

ignore .idea folder #522

merged 2 commits into from
Aug 31, 2016

Conversation

denofevil
Copy link
Contributor

this change is requred for IntelliJ-based products because project information files are written before actual generator is invoked

this change is requred for IntelliJ-based products because project information files are written before actual generator is invoked
@ghost
Copy link

ghost commented Aug 31, 2016

Thank you for your pull request and welcome to our community. We require contributors to sign our Contributor License Agreement, and we don't seem to have you on file. In order for us to review and merge your code, please sign up at https://code.facebook.com/cla - and if you have received this in error or have any questions, please drop us a line at cla@fb.com. Thanks!

@ghost
Copy link

ghost commented Aug 31, 2016

Thank you for signing our Contributor License Agreement. We can now accept your code for this (and any) Facebook open source project. Thanks!

@eliperelman
Copy link
Contributor

I wouldn't put this change in this project, as something like this is a developer-specific change which they would need to do for every project they worked on. I use Webstorm, and I have this in my ~/.gitignore_global file so I never have to worry about committing .idea to whatever repo I'm working on.

@eliperelman
Copy link
Contributor

Plus having an existing .idea directory is a good sign there has been some project already initialized in that directory.

@gaearon
Copy link
Contributor

gaearon commented Aug 31, 2016

This is not about gitignore, it’s about not bailing from creating a project when .idea exists.
It is needed for CRA + WebStorm integration, and WS team is committed to supporting it: #368 (comment)

@gaearon
Copy link
Contributor

gaearon commented Aug 31, 2016

To be clear this change is coming from WebStorm team because of how their project generator works: #368 (comment). I’m happy to accept other exceptions and special cases from IDE product teams who want to integrate with us.

@eliperelman
Copy link
Contributor

eliperelman commented Aug 31, 2016

Ah, I see, this is a tighter integration. I was confused as to why a .idea directory would exist in a fresh create-react-app project. My mistake, nothing to see here.

@gaearon
Copy link
Contributor

gaearon commented Aug 31, 2016

Yea, I’m not 100% sure about this but I trust them to know what they’re doing. 😄

@@ -171,7 +171,7 @@ function checkNodeVersion() {
// https://github.com/facebookincubator/create-react-app/pull/368#issuecomment-237875655
function isGitHubBoilerplate(root) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Let’s rename this to isSafeToCreateProjectIn(root).
Then add comment like

// If project only contains files generated by GH, it’s safe.
// We also special case WebStorm .idea because it integrates with CRA:
// https://github.com/facebookincubator/create-react-app/pull/368#issuecomment-243446094

@denofevil
Copy link
Contributor Author

denofevil commented Aug 31, 2016

@eliperelman I think that if there's only .idea it's ok. We are calling generator only from New Project dialog and if there's only .idea no harm would be caused on command-line too.

@gaearon gaearon merged commit a3f327a into facebook:master Aug 31, 2016
@gaearon
Copy link
Contributor

gaearon commented Aug 31, 2016

LGTM, thanks. I’ll cut a release this week.

@gaearon gaearon added this to the 0.3.0 milestone Aug 31, 2016
@denofevil
Copy link
Contributor Author

@gaearon thank you!

@gaearon gaearon mentioned this pull request Sep 1, 2016
stayradiated pushed a commit to stayradiated/create-react-app that referenced this pull request Sep 7, 2016
* ignore .idea folder
this change is requred for IntelliJ-based products because project information files are written before actual generator is invoked

* better method name and explanation
feiqitian pushed a commit to feiqitian/create-react-app that referenced this pull request Oct 25, 2016
* ignore .idea folder
this change is requred for IntelliJ-based products because project information files are written before actual generator is invoked

* better method name and explanation
@lock lock bot locked and limited conversation to collaborators Jan 22, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants