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

Cleanup and simplification patches #4

Open
wants to merge 9 commits into
base: master
Choose a base branch
from

Conversation

BartMassey
Copy link
Collaborator

Did a bunch of the stuff from my rewrite branch as small individual commits (except the rustfmt one is pretty big 🙂 ) so they can be reviewed separately. Please see the individual commits for the changes.

(Thanks for adding me as a committer. I still prefer to work through pull requests though: as you say, it's your codebase, plus review is always good.)

@BartMassey BartMassey changed the title Pr3 Cleanup and simplification patches Sep 6, 2019
@pistacchio
Copy link
Owner

I think we where working onthe same things (more or less). I added you as collaborator in order to be able to request your review on the PR I was working on!

I'll wait for PR #5 to be done before reviewing this.

As a side note: if you're interested, I'd love your eyes on this: https://github.com/pistacchio/rcrpg-rust

@BartMassey
Copy link
Collaborator Author

Sigh. Apologies, wish I'd known you were going to do some of this.

Really want the rustdoc commit. The other stuff is pretty redundant with what I did. Moving rng construction into Maze::new() makes sense.

Do you want to rebase these two patchsets together or shall I?

@pistacchio
Copy link
Owner

This PR now conflicts with master. Can you please clean this up to only include what you think is not redundant with my own fixes? Thanks!

@BartMassey
Copy link
Collaborator Author

I'll get to it at some point. It's going to mostly be cherry-picking patches out of my branch. My rustfmt should probably be thrown away and done over: I regret doing it now. Let's get things stabilized and then run rustfmt.

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