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

Adding some other ignores that aren't as obvious, but best practice #3777

Conversation

krainboltgreene
Copy link
Contributor

Added the secret_token.rb and *.yml files because they have sensitive
information.

Added .svn/ because you don't want to add other version control
directories to the repository.

Added test/tmp/ for the same reason tmp/ is ignored.

Added coverage/, .yardoc/, and rdoc/ because they generate a lot of
messy HTML files that are supposed to be generated on the fly

Added the secret_token.rb and *.yml files because they have sensitive
information.

Added .svn/ because you don't want to add other version control
directories to the repository.

Added test/tmp/ for the same reason tmp/ is ignored.

Added coverage/, .yardoc/, and rdoc/ because they generate a lot of
messy HTML files that are supposed to be generated on the fly
@josevalim
Copy link
Contributor

Thanks, please see comments below.

Added the secret_token.rb and *.yml files because they have sensitive
information.

If we ignore them, this means a recently created, pushed and then cloned project is not going to work at all. This should be done by project developers on their own (which hopefully will also add samples and setup task)

Added .svn/ because you don't want to add other version control
directories to the repository.

How frequent will this happen? This seems to be a rare case that should be handled individually.

Added test/tmp/ for the same reason tmp/ is ignored.

Again, it doesn't feel like this is very frequent.

Added coverage/, .yardoc/, and rdoc/ because they generate a lot of
messy HTML files that are supposed to be generated on the fly

I may be fine with adding coverage and rdoc. Rails doesn't not ship with .yardoc, so people adding yard to their projects should add it.

@masterkain
Copy link
Contributor

I'd add .DS_Store files to .gitignore by default :o)

@krainboltgreene
Copy link
Contributor Author

.yardoc

Good point.

test/tmp

Agreed, if test libraries are shoving into test/tmp they should be told to push to tmp/test to be more obvious.

.svn/, secret_token.rb, and *.yml

Perhaps these could be in a commented out "Commonly added" section, with some notes on why you want to ignore secret_token and database.yml.

I see all too many rails applications who don't do this, and it ends up biting them in the ass.

@josevalim
Copy link
Contributor

.DS_Store should be in your global gitignore.

@krainboltgreene
Copy link
Contributor Author

I've made the required changes, plus some. Let me know how it is.

@jeremy
Copy link
Member

jeremy commented Dec 16, 2011

Think a few commented examples are enough:

+# Examples
+#
+# Ignore generated documentation and code coverage:
+# /doc
+# /coverage
+#
+# Ignore a sensitive config file
+# /config/sensitive_config.yml

Examples of ignoring other version control is confusing, would exclude those.

@carlosantoniodasilva
Copy link
Member

Why ignoring the doc folder? It's supposed to contain app specific docs, I think that if you don't want it in a project, you should ignore it locally, but Rails should not ignore it by default.

@krainboltgreene
Copy link
Contributor Author

Because docs are generated static files, and since they should be generate'able at any time, to get the exact same state across any environment it should remain out of git.

If 5 developers can download a repository, run some generator, and get the same output, then that output shouldn't be added to the repository since it can't be "lost" and doesn't need versioning. If you version both the documentation and the code with the documentation you're versioning 1 thing twice, with the second wrapped in a ton of HTML and CSS.

@josevalim
Copy link
Contributor

-1 for ignoring the doc folder. I and many other developers use it to put project documentation, not automated documentation. In any case, I don't think this pull request is buying us something. We are adding ignores for files that are not necessarily generated in a Rails project (like coverage) and the current comments at the top pointing to github are enough. I would maybe just add a small comment about ignoring config/database.yml, but that is it. Thanks.

@stevenh512
Copy link
Contributor

I agree the doc folder should not be ignored, as @josevalim pointed out it's not necessarily only for automatically generated documentation. Perhaps doc/guides and doc/api might be better candidates, but even then, how often does someone generate those docs in their app? If I absolutely need to generate those docs (and most people don't, since most people have more reliable internet than I do lol) I run rails new docapp to create a bare app just for that purpose and add the necessary gems to the Gemfile (like RedCloth for guides) so I can keep it separate from any actual apps.

@vijaydev
Copy link
Member

vijaydev commented May 5, 2012

@krainboltgreene Please update this PR with the inputs of Jeremy and Jose, if you are still interested in getting this in. Thanks.

@steveklabnik
Copy link
Member

@krainboltgreene ping! Can you please update this, or else we'll have to reject it.

@steveklabnik
Copy link
Member

Can you please squash and remove the merge commit?

@vijaydev @fxn anything else we can do to get this moved forward?

@krainboltgreene
Copy link
Contributor Author

Removed the commit. You want me to swash all this into a single commit?

@fxn
Copy link
Member

fxn commented Sep 28, 2012

I have the same feeling as @josevalim, not seeing too much benefit in this proposal, these are very specific directories that should be added on a per project basis (or global ignores).

I would not even put config/database.yml since I have seen many projects that have that file in the repo.

If your config/database.yml does not go to the repo, then you add it to .gitignore. And of course you do it because otherwise git status is nagging you all the time. And people know what to do.

So, appreciate the work, but 👎 over here.

@rafaelfranca
Copy link
Member

I have to agree with @josevalim and @fxn.

Thank you for the work. Closing this now.

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.

None yet

10 participants