Remove obsolete junk from .gitignore #498

Be-ing · 2021-08-19T14:01:45Z

Resolves: stupid issues like https://github.com/tenacityteam/tenacity/pull/494#issuecomment-901845204

Checklist

I have signed off my commits using -s or Signed-off-by* (See: Contributing § DCO)
I made sure the code compiles on my machine
I made sure there are no unnecessary changes in the code*
I made sure the title of the PR reflects the core meaning of the issue you are solving*
I made sure the commit message(s) contain a description and answer the question "Why do those changes fix that particular issue?" or "Why are those changes really necessary as improvements?"*

* indicates required

leio · 2021-08-19T14:09:27Z

.gitignore

-# IDEA IDEs
-.idea/


Please keep this

Could have a slash in front though, for only ignoring at toplevel.

No, .gitignore is for files that should be ignored for everyone working on the project. Files that are created only by your own system should be ignored for you only:

git config --global core.excludesFile '.idea'

Sounds good, just the discoverability of this is so bad it has escaped me for over a decade as a good approach instead of listing them in project .gitignore. To the point it seems like something to note in some development docs of ours then, or we'll start getting PRs that try to add them all the time and we won't have anything to point them at as per instructions from our end.

Sure, could you make a pull request to add it to CONTRIBUTING.md?

I cannot recall seeing a pull request where someone accidentally included a file generated by their editor. If that ever does happen, it is easy to catch in code review and tell the author to run git config --global core.excludesFile so I don't see what the problem is.

I also think that this should be kept, I don't see any benefits of removing it

Keeping this here is a statement that these specific editors are preferred. I don't think that's appropriate.

But vcpkg_installed is also in the gitignore, so that means that I have to use vcpkg to install dependencies?

Keeping this here is a statement that these specific editors are preferred. I don't think that's appropriate.

On the contrary, if they were preferred, you'd let people ship relevant configurations of IDEs with them, rather than block them from doing so and "degrade" the integration with those specific IDEs, which is what we're doing here. If the .gitignore bore this sort of significance, it'd also be as if we were saying "we only prefer IDEs that leave junk in directories", which doesn't make too much sense.

Some repositories (especially the ones concerning JavaScript code, from what I've seen) leave IDE-specific directories on purpose in order to allow people to come up with a development environment instantly. This explicitly disallows people from doing so.

We could have a comment explaining that these editors are not preferred instead, maybe?

.gitignore

Signed-off-by: Be <be@mixxx.org>

n0toose

See https://github.com/tenacityteam/tenacity/pull/498#discussion_r693037356

space3375-ux · 2021-08-20T15:45:46Z

See #498 (comment)

n0toose · 2021-08-20T16:52:21Z

Hi, @AN7body! Welcome to GitHub! I'm not sure if there's something you wanted to say about the change, but it didn't go through, unfortunately.

emabrey · 2021-08-21T20:07:27Z

.gitignore

-*.obj
-*.pyc
-
-# Other unwanted files.


This entire section should not be removed.

emabrey · 2021-08-21T20:07:44Z

.gitignore

-*.bsc
-*.aps
-
-# Precompiled Headers


Neither should this one.

emabrey · 2021-08-21T20:09:44Z

.gitignore

-*.pch
-*.ncb
-
-# Compiled Dynamic libraries


Same. These aren't "junk" or "outdated". They serve as extremely low-cost safe-guards against accidents. I don't know why you would ever take such a hardline approach to git ignores. They cost almost nothing to maintain, nothing to check and they prevent accidents.

emabrey · 2021-08-21T20:13:36Z

.gitignore

-*.a
-*.lib
-
-# Executables


These are also useful safeguards.

emabrey · 2021-08-21T20:14:25Z

.gitignore

-# Compiled translation files (GNU Machine Object)
-*.gmo
-
-# Compiled Static libraries


Same here, but you could get rid of the top two and just leave .a and .lib

emabrey · 2021-08-21T20:15:29Z

.gitignore

-lib-src/twolame/simplefrontend/stwolame
-
-# Mac Specific
-.DS_Store


This should be left in. I believe Mac creates these automatically as hidden files to store folder attributes. Someone could literally commit this and have no idea it was even in the commit.

emabrey · 2021-08-21T20:16:13Z

.gitignore

@@ -198,21 +12,4 @@ help/temp*
 src/RevisionIdent.h
 win/resetPrefs.txt

-# Emacs backup files
-*~


These are files automatically created by emacs in a lot of cases and if anyone has their emacs setup to generate them it's very easy to accidentally commit them.

This is also done by some other editors too.

Yes, other editors do this too. Following your reasoning, we should add the files created by every editor out there into .gitignore... do you really think that's appropriate?

No, I meant some editors create ~ files too.

emabrey

Please see my other comments.

Be-ing · 2021-08-21T20:50:16Z

@emabrey all the build artifacts are ignored by ignoring the CMake build directory.

emabrey · 2021-08-21T21:15:00Z

@emabrey all the build artifacts are ignored by ignoring the CMake build directory.

Someone can just go cmake -B random-build-directory and the ignores don't apply. People are only forced to build out of the root of the tree, we aren't forcing them to build in a directory named build.

Be-ing · 2021-08-21T21:26:26Z

If someone uses a different name for their CMake build directory, I think it's kinda silly to attempt to add every single file within that to .gitignore. I have never seen people accidentally committing entire CMake build directories be a problem in practice. Also if someone deviates from the documented build instructions, I think it's on them to know what they're doing.

emabrey · 2021-08-21T21:31:39Z

If someone uses a different name for their CMake build directory, I think it's kinda silly to attempt to add every single file within that to .gitignore. I have never seen people accidentally committing entire CMake build directories be a problem in practice. Also if someone deviates from the documented build instructions, I think it's on them to know what they're doing.

But using the .gitignore like this is kind of already a standard practice. There are even popular websites/CLI that are literally setup to help you do it because people got tired of doing it so often:

https://www.toptal.com/developers/gitignore
https://github.com/toptal/gitignore.io
https://www.toptal.com/developers/gitignore/api/windows,macos,linux,python,cmake,c++,visualstudio,visualstudiocode,clion+all,emacs,vim,notepadpp,sublimetext,git

Be-ing · 2021-08-21T21:40:42Z

I have a really hard time taking something that uses a Docker container to setup a .gitignore file seriously.

Semisol

(If you are only frustrated about the *.pc part, make it more specific)
I see this as an overall negative change, and it will probably frustrate more people by committing in files generated by their OS or editor.

PS: Feel free to remove the lib-src .gitignores from devendored dependencies

Be-ing force-pushed the gitignore branch from 1ef0bc8 to 090bd33 Compare August 19, 2021 14:02

leio suggested changes Aug 19, 2021

View reviewed changes

remove obsolete junk from .gitignore

8d1b53a

Signed-off-by: Be <be@mixxx.org>

Be-ing force-pushed the gitignore branch from 090bd33 to 8d1b53a Compare August 19, 2021 14:21

leio approved these changes Aug 19, 2021

View reviewed changes

TheEvilSkeleton mentioned this pull request Aug 20, 2021

Add Flatpak manifest #494

Closed

5 tasks

n0toose suggested changes Aug 20, 2021

View reviewed changes

emabrey reviewed Aug 21, 2021

View reviewed changes

.gitignore

*.obj

*.pyc

# Other unwanted files.

Copy link

Member

emabrey Aug 21, 2021

Choose a reason for hiding this comment

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

This entire section should not be removed.

emabrey reviewed Aug 21, 2021

View reviewed changes

.gitignore

*.bsc

*.aps

# Precompiled Headers

Copy link

Member

emabrey Aug 21, 2021

Choose a reason for hiding this comment

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

Neither should this one.

emabrey reviewed Aug 21, 2021

View reviewed changes

.gitignore

*.a

*.lib

# Executables

Copy link

Member

emabrey Aug 21, 2021

Choose a reason for hiding this comment

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

These are also useful safeguards.

emabrey reviewed Aug 21, 2021

View reviewed changes

emabrey suggested changes Aug 21, 2021

View reviewed changes

emabrey changed the title ~~remove obsolete junk from .gitignore~~ Remove obsolete junk from .gitignore Aug 21, 2021

Semisol self-requested a review August 22, 2021 04:30

Semisol suggested changes Aug 22, 2021

View reviewed changes

n0toose marked this pull request as draft September 16, 2021 09:59

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Remove obsolete junk from .gitignore #498

Remove obsolete junk from .gitignore #498

Be-ing commented Aug 19, 2021

leio Aug 19, 2021

leio Aug 19, 2021

Be-ing Aug 19, 2021

leio Aug 19, 2021 •

edited

Be-ing Aug 19, 2021

Be-ing Aug 20, 2021

AnotherFoxGuy Aug 20, 2021

Be-ing Aug 20, 2021

AnotherFoxGuy Aug 20, 2021

n0toose Aug 20, 2021

n0toose left a comment

space3375-ux commented Aug 20, 2021

n0toose commented Aug 20, 2021

emabrey Aug 21, 2021

emabrey Aug 21, 2021

emabrey Aug 21, 2021

emabrey Aug 21, 2021

emabrey Aug 21, 2021

emabrey Aug 21, 2021

emabrey Aug 21, 2021

Semisol Aug 22, 2021

Be-ing Aug 22, 2021

Semisol Aug 22, 2021

emabrey left a comment

Be-ing commented Aug 21, 2021

emabrey commented Aug 21, 2021

Be-ing commented Aug 21, 2021

emabrey commented Aug 21, 2021 •

edited

Be-ing commented Aug 21, 2021

Semisol left a comment

Remove obsolete junk from .gitignore #498

Are you sure you want to change the base?

Remove obsolete junk from .gitignore #498

Conversation

Be-ing commented Aug 19, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

leio Aug 19, 2021 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

n0toose left a comment

Choose a reason for hiding this comment

space3375-ux commented Aug 20, 2021

n0toose commented Aug 20, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

emabrey left a comment

Choose a reason for hiding this comment

Be-ing commented Aug 21, 2021

emabrey commented Aug 21, 2021

Be-ing commented Aug 21, 2021

emabrey commented Aug 21, 2021 • edited

Be-ing commented Aug 21, 2021

Semisol left a comment

Choose a reason for hiding this comment

leio Aug 19, 2021 •

edited

emabrey commented Aug 21, 2021 •

edited