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

Refactor the runtime settings into a struct #1

Merged
merged 1 commit into from
Nov 24, 2016

Conversation

perlun
Copy link
Owner

@perlun perlun commented Nov 24, 2016

This makes more sense than having a lot of parameters. Making this change makes it possible to do a lot of other nice changes, will probably continue doing one of these as soon as I get this reviewed. :)

/cc @mpalmujoki, can't assign you since you haven't accepted the invite yet.

This makes more sense than having a lot of parameters. Making this change makes it possible to do a lot of other nice changes, will probably continue doing one of these as soon as I get this reviewed. :)
@perlun perlun force-pushed the refactor/move-runtime-settings-to-struct branch from 8e3e954 to e4da106 Compare November 24, 2016 20:32
@perlun perlun merged commit 27ee48d into master Nov 24, 2016
@perlun perlun deleted the refactor/move-runtime-settings-to-struct branch November 24, 2016 20:39
Copy link
Collaborator

@mpalmujoki mpalmujoki left a comment

Choose a reason for hiding this comment

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

Don't really know what to comment on here.

to_revision: &'a str
}

impl<'a> Settings<'a> {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Tried to look up the impl thing, but <'a> Settings<'a> leaves me flustered.

Copy link
Owner Author

Choose a reason for hiding this comment

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

Understandable. 😄 For reading: Lifetimes in the Rust Book has all the information you are wondering about.

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

Successfully merging this pull request may close these issues.

2 participants