Skip to content

Fix gitignores #592

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

Merged
merged 2 commits into from
Aug 8, 2018
Merged

Fix gitignores #592

merged 2 commits into from
Aug 8, 2018

Conversation

lutostag
Copy link
Contributor

Found that some of the exercises had different gitignores. Caused me to almost commit the /target/ files to my exercises when running through them (reverse string was the first I ran into with the problem).

Made sure they all have the same now:

# Generated by Cargo
# will have compiled files and executables
/target/
**/*.rs.bk

# Remove Cargo.lock from gitignore if creating an executable, leave it for libraries
# More information here http://doc.crates.io/guide.html#cargotoml-vs-cargolock
Cargo.lock

Also updated the init_exercise.py to have the same gitignore for future created projects.

You can check they are all the same (except for grep) which I also included the *.txt because it seemed like some of the tests may have large text files there... by running
md5sum exercises/*/.gitignore | sort

Hope this is helpful.

Thanks!

Copy link
Member

@coriolinus coriolinus left a comment

Choose a reason for hiding this comment

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

Thanks, @lutostag, for doing this work!

In general, we prefer to keep the scope of each PR separate. It's better to have one PR which updates the necessary readme files, and another which must be rebased after the first is merged, than to have multiple unrelated changes in a single PR. See also here for another track maintainer's perspective.

As such, please remove the changes to non-.gitignore files from this PR. If this causes Travis to fail, then so be it; Travis is advisory, not mandatory, when choosing whether or not to merge.

@@ -13,6 +13,8 @@ anything.

He answers 'Whatever.' to anything else.

Bob's conversational partner is a purist when it comes to written communication and always follows normal rules regarding sentence punctuation in English.
Copy link
Member

Choose a reason for hiding this comment

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

This has already been updated by #597. Please remove this change from this PR.

Now it will include `target` compiled files as well.
@lutostag
Copy link
Contributor Author

lutostag commented Aug 4, 2018

@coriolinus Thanks for the helpful pointers, I should have checked the PRs first -- oops...

I removed the extraneous changes, cleaned history up and rebased on latest master.
(I tried to follow the commit guidelines as well -- missed that first time around).

Let me know if anything else needs fixing. Thanks!

Copy link
Member

@coriolinus coriolinus left a comment

Choose a reason for hiding this comment

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

Thanks! There's one more change; you overwrote a .gitignore which had already been customized. After this, I think we'll be all set.

# More information here http://doc.crates.io/guide.html#cargotoml-vs-cargolock
Cargo.lock
*.txt
Copy link
Member

Choose a reason for hiding this comment

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

This line should not be removed; the grep exercise generates some text files, and while it is meant to clean up after itself, it might not in the event of panics.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's moved to the top line. Still there. I'll move it back to the bottom

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Using your explanation as a comment there too. Thanks!

The grep .gitignore also ignores large *.txt files
All the rest now ignore target and Cargo.lock as they should
Also ignoring **/*.rs.bk as that was in some but not all.

Can be verified with: `md5sum exercises/*/.gitignore | sort`
Copy link
Member

@coriolinus coriolinus left a comment

Choose a reason for hiding this comment

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

Thanks for doing this, @lutostag! Next steps: I'm going to leave this open for a while to give the other track maintainers the option to take a look at it and make comments as desired.

Assuming there are no objections, I intend to merge this not later than Wednesday the 8th. If on Thursday I haven't yet done so, please feel free to ping me a reminder.

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.

2 participants