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

Stop sorting config keys for newer ruby versions. #13

Merged
merged 3 commits into from
Jan 28, 2016

Conversation

gizmoguy
Copy link
Contributor

  • For ruby versions > 1.9.3 use order specified by user for
    configuration, otherwise sort it alphabetically.
  • Remove extraneous whitespace.

 * For ruby versions > 1.9.3 use order specified by user for
   configuration, otherwise sort it alphabetically.
 * Remove extraneous whitespace.
@gizmoguy
Copy link
Contributor Author

Sorry submitting on a different PR to #12 but I messed up my rebase.

I've performed the changes asked for and added a bunch of tests because I didn't have any machines to test on with older ruby versions.

I'll also look into fixing the lint issues picked up by rubocop.

@edestecd
Copy link
Collaborator

Other then the rubocop errors, it looks good to me.

 * For ruby versions > 1.9.3 use order specified by user for
   configuration, otherwise sort it alphabetically.
 * Remove extraneous whitespace.
@gizmoguy
Copy link
Contributor Author

Fixed most of them.

What do we think about "Style/RegexpLiteral: Use %r around regular expression.".

Usually I prefer using slashes as the regexp delimiter?

@edestecd
Copy link
Collaborator

I always go with the community recommendation. Unless you have a really good reason not to.

@edestecd
Copy link
Collaborator

Its much cleaner if you don't have to use the escapes. Escapes make it harder to read.

 * For ruby versions > 1.9.3 use order specified by user for
   configuration, otherwise sort it alphabetically.
 * Remove extraneous whitespace.
@gizmoguy
Copy link
Contributor Author

Force of habit really, which isn't a good enough reason.

I have now fixed all rubocop issues and tests seem to all pass now.

@edestecd
Copy link
Collaborator

Super. Thanks for the contribution!

@edestecd edestecd closed this Jan 28, 2016
@edestecd edestecd reopened this Jan 28, 2016
edestecd added a commit that referenced this pull request Jan 28, 2016
Stop sorting config keys for newer ruby versions.
@edestecd edestecd merged commit 8539c01 into sgnl05:master Jan 28, 2016
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