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

chore(gitignore): remove no related project files #613

Merged
merged 3 commits into from
Jun 10, 2017
Merged

chore(gitignore): remove no related project files #613

merged 3 commits into from
Jun 10, 2017

Conversation

RomainLanz
Copy link
Contributor

@RomainLanz RomainLanz commented Apr 27, 2017

Hey everyone !

.idea is editor related and .DS_STORE is OS related. Both of those files/folders should be inside a global gitignore file since they are not related to the project.

scripts folder is recreate by the CLI when you bundle your application, there's no need to push compiled file so it's better to remove them by default.

`.idea` is editor related and `.DS_STORE` is OS related. Both of those files/folders should be inside a global gitignore file since they are not related to the project.

`scripts` folder is recreate by the CLI when you bundle your application, there's no need to push compiled file so it's better to remove them by default.
@CLAassistant
Copy link

CLAassistant commented Apr 27, 2017

CLA assistant check
All committers have signed the CLA.

@JeroenVinke
Copy link
Collaborator

JeroenVinke commented Apr 29, 2017

Hey @RomainLanz, thanks for this PR!

Unless there is a particular reason for not doing so, i'd be in favor of adding the scripts folder to the gitignore as well. But i'm not sure about the editor configs. I can see how some people would like to have editor configs in the repository, but some don't.

@AStoker what do you think?

@RomainLanz
Copy link
Contributor Author

I don't have editor configs in my repository but they are not in the project .gitignore since it's not related to the project itself. That's why you can create a global gitignore to ignore all files related to your computer/editor (Thumb.db, .DS_STORE, .idea, .vscode, ...)

@AStoker
Copy link
Contributor

AStoker commented May 1, 2017

I'm with @JeroenVinke with adding the scripts to the list, makes sense.
In regards to the other files, I'm inclined to leave them in there, unless there's a specific reason to remove them (are they causing a problem?). I don't think it hurts having it (potentially) in two places (global and local), and it's convenient that it's just added to the project (don't have to go modifying a global). I actually usually check in my .vscode folder since there's things in there I want across a few private repos to keep workstations similar.

@RomainLanz
Copy link
Contributor Author

Well if you decide to keep editor/computer related files why not add Thumb.db, .DS_STORE, .idea, etc. ?

@AStoker
Copy link
Contributor

AStoker commented May 2, 2017

I'm personally not opposed to it.
Many people don't bother with adding a global git excludes, and so while having it here would be redundant, I see it helping more by leaving it here than by removing it. @JeroenVinke, what are your thoughts?

@JeroenVinke
Copy link
Collaborator

JeroenVinke commented May 2, 2017

Many people don't bother with adding a global git excludes, and so while having it here would be redundant, I see it helping more by leaving it here than by removing it.

Since the CLI is all about getting a working setup quickly (that you can push to github right away), I'd like to keep the editor/OS related files in the gitignore., so I agree with @AStoker. @RomainLanz could you add those editor/computer related files you mentioned, in addition to the scripts folder you already added?

@RomainLanz
Copy link
Contributor Author

Yes, so which file should I add? .vscode, .idea, Thumb.db, .DS_STORE?

Should I also write a little comment on the file linking to this page if they want an advanced configuration?

@JeroenVinke
Copy link
Collaborator

That would be good 👍

@RomainLanz
Copy link
Contributor Author

Okay.

I have updated the file, what do you think? @JeroenVinke @AStoker

Copy link
Contributor

@AStoker AStoker left a comment

Choose a reason for hiding this comment

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

👍

@JeroenVinke JeroenVinke merged commit 1aebdb6 into aurelia:master Jun 10, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants