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

REF: Moved mkrf_conf error to errors lib #30

Merged
merged 2 commits into from
Apr 23, 2019
Merged

REF: Moved mkrf_conf error to errors lib #30

merged 2 commits into from
Apr 23, 2019

Conversation

venarius
Copy link
Contributor

Hey 👋

Since this project has a .rubocop.yml file, I guessed that doing some rubocop refactoring wouldn't be bad. Thats what I did here. Also extracted the mkrf_conf errors to the lib/errors.rb

Together with #29 all rubocop warnings / errors should be fixed now 🙂

@matthewd
Copy link
Member

Hi!

Thanks for this!

Could you possibly split the error change into a separate PR?

That one's an easy merge, whereas the rubocop fixes -- which would normally be even easier -- will probably create conflicts with #27, so I'd rather hold off on those until I've decided whether I can stomach Standard's formatting predilections.

@venarius
Copy link
Contributor Author

Yes sure, I will convert this to be only for the error.

@venarius
Copy link
Contributor Author

@matthewd just changed this commit :)

@venarius venarius changed the title REF: Rubocop refactoring and extracted error REF: Moved mkrf_conf error to errors lib Apr 22, 2019
lib/gel/error.rb Outdated Show resolved Hide resolved
lib/gel/package/installer.rb Outdated Show resolved Hide resolved
@venarius
Copy link
Contributor Author

I fixed the typo, changed the command name to be more broad and also added a little test that checks for the NoGemfile error :)

@matthewd
Copy link
Member

Great, thanks!

(Though the flaw in the pattern is that it was already working -- I haven't actually checked, but I'm 95% sure that test would pass with the typo present.)

@matthewd matthewd merged commit d60a1b4 into gel-rb:master Apr 23, 2019
@venarius venarius deleted the rubocop branch April 23, 2019 19:27
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