Skip to content

Refactoring for easier reuse, merge CLI and config #74

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 8 commits into from
Jan 23, 2016

Conversation

nochso
Copy link
Contributor

@nochso nochso commented Jan 17, 2016

See #72

Mostly trying to shrink down all the boiler-plate in CompareCommand's execute method.

What do you think so far? @tomzx

See commit "Merge configuration and CLI input" for implementing #67.

@nochso nochso changed the title Refactoring for easier reuse Refactoring for easier reuse, merge CLI and config Jan 17, 2016
@nochso
Copy link
Contributor Author

nochso commented Jan 18, 2016

Thanks for the feedback. Should I push fixes to this PR, rewrite this one or create a new clean PR..?

@tomzx
Copy link
Owner

tomzx commented Jan 19, 2016

I think it's generally usage on github to simply rebase your work (aka overwrite what you've done so far and simply push on the same branch which should update this PR).

@nochso nochso force-pushed the refactor branch 2 times, most recently from 60213c2 to 9ee2af8 Compare January 20, 2016 21:28
@nochso
Copy link
Contributor Author

nochso commented Jan 20, 2016

Ok, I reworked the PR. Merging is now done by an InputMerger.

@tomzx tomzx added this to the Candidate for next Minor milestone Jan 20, 2016
@tomzx
Copy link
Owner

tomzx commented Jan 22, 2016

I'll give this a test run and I'll try to merge this within the day.

Thanks for the work!

@tomzx tomzx merged commit e7e6216 into tomzx:master Jan 23, 2016
@tomzx
Copy link
Owner

tomzx commented Jan 23, 2016

Thanks again!

@nochso
Copy link
Contributor Author

nochso commented Jan 23, 2016

Great, thank you!

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