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.

Sign up for GitHub

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

Jump to bottom

gitignore: add osx .DS_STORE files #10052

Closed
wants to merge 1 commit into from
Closed

gitignore: add osx .DS_STORE files #10052

wants to merge 1 commit into from

Conversation

sarahmeyer
Copy link
Contributor

Checklist
  • make -j8 test (UNIX), or vcbuild test nosign (Windows) passes
  • commit message follows commit guidelines
Affected core subsystem(s)

project gitignore

Description of change

Obviously, people can and should add .DS_STORE files to their global gitignore, but I don't think it'll hurt anyone to have it in the project file.

@nodejs-github-bot nodejs-github-bot added the meta Issues and PRs related to the general management of the project. label Dec 1, 2016
@kenany
Copy link
Contributor

kenany commented Dec 1, 2016

Previous PRs: #159, #3357

@Trott
Copy link
Member

Trott commented Dec 2, 2016

Thanks for doing this! FYI, there is also a relevant PR open to specify the dot-files we want in .gitignore and ignore all others: #8016

@imyller
Copy link
Member

imyller commented Dec 2, 2016

If #8016 is no longer stalled then this issue would be better solved there - if not, I personally think that .DS_Store should be in .gitignore.

@bnoordhuis
Copy link
Member

This is similar to those pull requests that added ignore rules for editor files: once you accept one, you have to accept them all. I personally don't see the value.

@Fishrock123
Copy link
Contributor

Fishrock123 commented Dec 2, 2016 •
edited
Loading

@bnoordhuis I do agree (and I think we did at the code&learn too). I wonder that this was not added previously given the history though.

Should we just clean this up sometime and message better to people that they should (and how to) globally gitignore?

@gibfahn
Copy link
Member

gibfahn commented Dec 3, 2016 •
edited
Loading

@Fishrock123 If #8016 lands, I think that would fix this issue (and other similar ones).

@Fishrock123
Copy link
Contributor

@gibfahn Ah that is a much better solution, yes. Let's close this once that lands.

@sarahmeyer #8016 is a much more comprehensive solution, but thanks for reminding us that this is/was still present. (I think you said you were on a new laptop? I suppose that might be why you didn't have it globally. :P)

@Fishrock123 Fishrock123 self-assigned this Dec 3, 2016
Fishrock123 pushed a commit that referenced this pull request Dec 5, 2016
@claudiorodriguez @Fishrock123
meta: whitelist dotfiles in .gitignore
15cc7c0 
Instead of excluding IDE-specific dotfiles, exclude all and
then whitelist those the project needs to track.

Refs: #8010
Refs: #9111
Refs: #10052

Fixes: #8012
PR-URL: #8016

Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Franziska Hinkelmann <franziska.hinkelmann@gmail.com>
Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Roman Reiss <me@silverwind.io>
Reviewed-By: Josh Gavant <josh.gavant@outlook.com>
@Fishrock123
Copy link
Contributor

Solved by 15cc7c0 :D

@Fishrock123 Fishrock123 closed this Dec 5, 2016
addaleax pushed a commit that referenced this pull request Dec 5, 2016
@claudiorodriguez @addaleax
meta: whitelist dotfiles in .gitignore
3253954 
Instead of excluding IDE-specific dotfiles, exclude all and
then whitelist those the project needs to track.

Refs: #8010
Refs: #9111
Refs: #10052

Fixes: #8012
PR-URL: #8016

Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Franziska Hinkelmann <franziska.hinkelmann@gmail.com>
Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Roman Reiss <me@silverwind.io>
Reviewed-By: Josh Gavant <josh.gavant@outlook.com>
jmdarling pushed a commit to jmdarling/node that referenced this pull request Dec 8, 2016
@claudiorodriguez
meta: whitelist dotfiles in .gitignore
96ade1f 
Instead of excluding IDE-specific dotfiles, exclude all and
then whitelist those the project needs to track.

Refs: nodejs#8010
Refs: nodejs#9111
Refs: nodejs#10052

Fixes: nodejs#8012
PR-URL: nodejs#8016

Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Franziska Hinkelmann <franziska.hinkelmann@gmail.com>
Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Roman Reiss <me@silverwind.io>
Reviewed-By: Josh Gavant <josh.gavant@outlook.com>
MylesBorins pushed a commit that referenced this pull request Dec 20, 2016
@claudiorodriguez @MylesBorins
meta: whitelist dotfiles in .gitignore
65e134f 
Instead of excluding IDE-specific dotfiles, exclude all and
then whitelist those the project needs to track.

Refs: #8010
Refs: #9111
Refs: #10052

Fixes: #8012
PR-URL: #8016

Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Franziska Hinkelmann <franziska.hinkelmann@gmail.com>
Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Roman Reiss <me@silverwind.io>
Reviewed-By: Josh Gavant <josh.gavant@outlook.com>
MylesBorins pushed a commit that referenced this pull request Dec 20, 2016
@claudiorodriguez @MylesBorins
meta: whitelist dotfiles in .gitignore
0be56cd 
Instead of excluding IDE-specific dotfiles, exclude all and
then whitelist those the project needs to track.

Refs: #8010
Refs: #9111
Refs: #10052

Fixes: #8012
PR-URL: #8016

Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Franziska Hinkelmann <franziska.hinkelmann@gmail.com>
Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Roman Reiss <me@silverwind.io>
Reviewed-By: Josh Gavant <josh.gavant@outlook.com>
MylesBorins pushed a commit that referenced this pull request Dec 21, 2016
@claudiorodriguez @MylesBorins
meta: whitelist dotfiles in .gitignore
dc2e8a7 
Instead of excluding IDE-specific dotfiles, exclude all and
then whitelist those the project needs to track.

Refs: #8010
Refs: #9111
Refs: #10052

Fixes: #8012
PR-URL: #8016

Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Franziska Hinkelmann <franziska.hinkelmann@gmail.com>
Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Roman Reiss <me@silverwind.io>
Reviewed-By: Josh Gavant <josh.gavant@outlook.com>
MylesBorins pushed a commit that referenced this pull request Dec 21, 2016
@claudiorodriguez @MylesBorins
meta: whitelist dotfiles in .gitignore
b0cbc76 
Instead of excluding IDE-specific dotfiles, exclude all and
then whitelist those the project needs to track.

Refs: #8010
Refs: #9111
Refs: #10052

Fixes: #8012
PR-URL: #8016

Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Franziska Hinkelmann <franziska.hinkelmann@gmail.com>
Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Roman Reiss <me@silverwind.io>
Reviewed-By: Josh Gavant <josh.gavant@outlook.com>
@Trott Trott mentioned this pull request Oct 12, 2018
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@imyller imyller imyller approved these changes

Assignees

@Fishrock123 Fishrock123

Labels
meta Issues and PRs related to the general management of the project.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
8 participants
@sarahmeyer @kenany @Trott @imyller @bnoordhuis @Fishrock123 @gibfahn @nodejs-github-bot