Skip to content

Added .idea/ to gitignore, to cover cases where people are using a Jetbrains text editor#1497

Merged
pganssle merged 1 commit into
pypa:masterfrom
joeflack4:feature_gitignore_idea
Sep 20, 2018
Merged

Added .idea/ to gitignore, to cover cases where people are using a Jetbrains text editor#1497
pganssle merged 1 commit into
pypa:masterfrom
joeflack4:feature_gitignore_idea

Conversation

@joeflack4

Copy link
Copy Markdown
Contributor

Summary of changes

Added .idea/ to gitignore, to cover cases where people are using a Jetbrains text editor

@pganssle pganssle merged commit eaa86ef into pypa:master Sep 20, 2018
@pganssle

Copy link
Copy Markdown
Member

@joeflack4 Thanks for this PR! Always nice when you don't have to think too hard about whether or not to merge something ;)

@jaraco

jaraco commented Sep 20, 2018

Copy link
Copy Markdown
Member

Sorry to be a contrarian, but I've probably rejected this idea before. It doesn't make sense to update every project's .gitignore for every developer's environment. This preference should be added to each JetBrains developers global ignores (for example, here's mine). I also encourage Python developers to put Python ignores there. In my opinion, .gitignore for a project should include ignores unique to that project (and little more).

I won't revert this change. I've stopped trying to keep the ignores trim. Just wanted to share my opinion for perhaps a different perspective.

@pganssle

pganssle commented Sep 20, 2018

Copy link
Copy Markdown
Member

@jaraco I did consider this, and honestly I'd probably like to reorganize the .gitignore so it's clear what things should and shouldn't be in there and why each rule is inplace. Though we also do have some emacs-specific stuff in .gitignore as well, so this isn't the first "editor detritus" rule.

The reasons I think it's probably not so bad are:

  1. We definitely know we don't want to check in a file called .idea at any point in the future. We could ask everyone to configure their git correctly (and ideally they would do that as well because not every project will add every stupid editor file to their .gitignore), but this guarantees that if someone does it wrong, we don't have to bother removing it or telling them to remove it.
  2. It doesn't hurt anything (at least not at the moment) - again we never would want to check this file in, and even if we added the "editor detritus" for the top 50 editors, we'd probably never notice an impact in readability of the .gitignore or the speed of git commands.

Ideally editors and file systems wouldn't pollute working directories with .project or .idea or .DS_STORE or folder.jpg or whatever, and even if they did ideally people would configure their environments correctly, I agree, but it's probably easier to do this once than to deal with it every time it comes up.

@joeflack4

Copy link
Copy Markdown
Contributor Author

Thanks very much for the merge. I didn't expect this fast of a response, and these high quality comments in reply to the humble PR. Thanks!

@jaraco Also, I didn't even know about Jetbrains global ignores. Did you mean to have linked this though? If so, I do not see where the example is.

Also on other .gitignore variations, there's also this as a possibility?:

# syntax: glob
.*
!.codecov.yml
!.github/
!.gitignore
!.readthedocs.yml
!.readthedocs.yml
bin
build
dist
docs/build
include
lib
distribute.egg-info
setuptools.egg-info
*.egg
*.py[cod]
*.swp
*~

@jaraco

jaraco commented Sep 21, 2018

Copy link
Copy Markdown
Member

Did you mean to have linked this?

No; I'll correct corrected.

@jaraco

jaraco commented Sep 21, 2018

Copy link
Copy Markdown
Member

FYI, I cut a release with this change just to do a demo of the release workflow. I wouldn't normally cut a release for a change that doesn't affect the published distribution.

@joeflack4

Copy link
Copy Markdown
Contributor Author

Thanks @jaraco ! I'll use this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants