Skip to content

Added possibility to give opts via symbols instead of string as key for the opts hash#365

Open
AlbinOS wants to merge 3 commits intoupserve:masterfrom
GoPex:use_symbol_as_default_key_for_opts_hash
Open

Added possibility to give opts via symbols instead of string as key for the opts hash#365
AlbinOS wants to merge 3 commits intoupserve:masterfrom
GoPex:use_symbol_as_default_key_for_opts_hash

Conversation

@AlbinOS
Copy link
Contributor

@AlbinOS AlbinOS commented Feb 3, 2016

Pull request for #360.

I started writing test but it felt wrong as I was only duplicating existing test and calling methods with ':key' instead of '"key"'. Is it ok for you ? Or do you have a better idea to test theses changes in a smart way ?

@AlbinOS
Copy link
Contributor Author

AlbinOS commented Feb 5, 2016

Do you have any idea why is the build of my PR failing ? I ran all tests locally and everything passed normally.

@tlunter
Copy link
Contributor

tlunter commented Mar 22, 2016

Any reason you gave preference to the symbols approach instead of strings approach? Since strings were previously default, probably a good idea to keep that first.

@AlbinOS
Copy link
Contributor Author

AlbinOS commented Mar 29, 2016

@tlunter, not really, it's just an habit I guess. I'll modify this PR according to your remark. Thanks for the feedback !

@AlbinOS
Copy link
Contributor Author

AlbinOS commented Mar 30, 2016

@tlunter, it should be ok now.

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