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

Allow tags as hashes #107

Merged
merged 3 commits into from
Jun 14, 2019
Merged

Conversation

jtzemp
Copy link
Contributor

@jtzemp jtzemp commented Jan 31, 2019

I wanted the internal API to be more friendly to the Ruby "principle of least surprise." I'd like to be able to pass a hash to :tags when measuring things, instead of an array of strings delimited by a colon.

This change allows a user to pass tags into the .new() method as well as individual measurement methods.

I created specs that test the outside interfaces, but I didn't add any for the internal, private method I created. I can create a couple of unit tests for that method if you'd like.

@jtzemp
Copy link
Contributor Author

jtzemp commented Feb 20, 2019

Ping. Is there anything I can do to make this easier to merge? I'm happy to sign CLA.

I hereby license this code under the MIT licence (the same as this library). And/or whatever other licence might make it easier to integrate.

@allcentury
Copy link

I was just about to start a PR on this and then saw yours! Thanks @jtzemp

@masci masci added this to the Next milestone Apr 11, 2019
@albertvaka
Copy link
Contributor

Hi @jtzemp! Sorry we didn't review this in 6 months... 🙁

The patch looks good and the change makes a lot of sense, so merging!

Thanks a lot for your contribution (and your patience) 😀

@albertvaka albertvaka merged commit 16fda84 into DataDog:master Jun 14, 2019
pudiva pushed a commit to pudiva/dogstatsd-ruby that referenced this pull request Jun 5, 2023
README: Update for re-org and remove extra info
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.

4 participants