Skip to content

ignore .idea folder - #522

Merged
gaearon merged 2 commits into
react:masterfrom
denofevil:ignore_dot_idea
Aug 31, 2016
Merged

ignore .idea folder#522
gaearon merged 2 commits into
react:masterfrom
denofevil:ignore_dot_idea

Conversation

@denofevil

@denofevildenofevil commented Aug 31, 2016

Copy link
Copy Markdown
Contributor

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

@denofevil
ignore .idea folder
5ab129e
this change is requred for IntelliJ-based products because project information files are written before actual generator is invoked
@ghost

ghost commented Aug 31, 2016

Copy link
Copy Markdown

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!

@denofevildenofevil mentioned this pull request Aug 31, 2016
Merged
@ghostghost added the CLA Signed label Aug 31, 2016
@ghost

ghost commented Aug 31, 2016

Copy link
Copy Markdown

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

@eliperelman

eliperelman commented Aug 31, 2016

Copy link
Copy Markdown
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

eliperelman commented Aug 31, 2016

Copy link
Copy Markdown
Contributor

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

@gaearon

gaearon commented Aug 31, 2016

Copy link
Copy Markdown
Contributor

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

gaearon commented Aug 31, 2016

Copy link
Copy Markdown
Contributor

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

eliperelman commented Aug 31, 2016•
edited
Loading

Copy link
Copy Markdown
Contributor

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

gaearon commented Aug 31, 2016

Copy link
Copy Markdown
Contributor

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

gaearon
gaearon reviewed Aug 31, 2016
View reviewed changes
Comment threadglobal-cli/index.js Outdated
@@ -171,7 +171,7 @@ function checkNodeVersion() {
// https://github.com/facebookincubator/create-react-app/pull/368#issuecomment-237875655
function isGitHubBoilerplate(root) {

@gaearongaearonAug 31, 2016

Copy link
Copy Markdown
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

denofevil commented Aug 31, 2016•
edited
Loading

Copy link
Copy Markdown
ContributorAuthor

@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 react:masterAug 31, 2016
@gaearon

gaearon commented Aug 31, 2016

Copy link
Copy Markdown
Contributor

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

@gaearongaearon added this to the 0.3.0 milestone Aug 31, 2016
@denofevil

denofevil commented Aug 31, 2016

Copy link
Copy Markdown
ContributorAuthor

@gaearon thank you!

@gaearongaearon mentioned this pull request Sep 1, 2016
Merged
stayradiated pushed a commit to stayradiated/create-react-app that referenced this pull request Sep 7, 2016
@denofevilGeorge Czabania
ignore .idea folder (react#522)
eb85f24
* 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
@denofevil@feiqitian
ignore .idea folder (react#522)
7b86b81
* 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
@locklockBot locked and limited conversation to collaborators Jan 22, 2019
Sign up for freeto subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

No reviews

Assignees

No one assigned

Labels

CLA Signed

Projects

None yet

Milestone

0.3.0

Development

Successfully merging this pull request may close these issues.

3 participants

@denofevil@eliperelman@gaearon