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

Use Ruby 1.9+ hash literal syntax #76

Closed
wants to merge 3 commits into from

Conversation

dcarral
Copy link

@dcarral dcarral commented Sep 17, 2016

;)

@randycoulman
Copy link
Contributor

Thanks for the contribution, @dcarral! I, too, prefer the 1.9 hash syntax over hash rockets. However, the rest of the code base uses hashrockets consistently, so I think we should keep the README as-is for now.

I'll keep this PR open for a little while to give @kytrinyx a chance to weigh in.

@dcarral
Copy link
Author

dcarral commented Sep 17, 2016

@randycoulman Thanks a lot for the instant feedback! :) It makes sense. What do you think about changing to the new syntax in the codebase too, then?

@dcarral dcarral changed the title Use Ruby 1.9 hash literal syntax @ README Use Ruby 1.9+ hash literal syntax Sep 17, 2016
@kytrinyx
Copy link
Contributor

Hi @dcarral thanks so much for taking the time to make this change.

While I don't feel particularly strongly about one syntax over another (e.g. I use tabs in Go and spaces in Ruby), I do feel strongly about consistency.

With hashes, the colon syntax only applies to keys that are symbols... and only symbols that are alphanumeric. If we use the colon syntax and need other types of keys, or keys that have hyphens, then this will introduce inconsistency, which has an added cost when reading it.

For this reason I would prefer to leave the hash syntax as is.

@dcarral
Copy link
Author

dcarral commented Sep 18, 2016

Thank you for the prompt feedback Katrina! ;)

I definitely understand your point regarding the usage of the old hash literal syntax in the codebase.

However, in my humble opinion, we shouldn't prevent the README from being updated to the new hash literal syntax. Most Ruby projects nowadays use the new syntax (gut feeling, I don't have data to back up this -yet-), and keeping the old one in the README somehow gives the impression of unmaintained/out-of-date. Notice that consumers of the library don't need to follow the coding style used in the internals of it, for obvious reasons (n libraries, n coding styles).

Having said that, there are more (and more important) issues with the current status of the README, so probably addressing them first could mitigate the "not as fresh as expected" impression which, at least for me, the README gives.

Hope to find some time during this next week to keep polishing the README. I'm gonna need your feedback, so expect a couple of issues/PRs popping up soon to ask for it. Meanwhile... happy hacking! ;)

@randycoulman
Copy link
Contributor

Hi, @dcarral. We're super open to improvements to the README. See also #25 that has some additional ideas.

But for this particular change, we're going to keep things as-is to maintain the consistency that @kytrinyx talked about.

I'm going to close this PR, but as I said, other README improvements are most welcome. Thanks again for taking the time!

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.

3 participants