Skip to content

Conversation

chad-codes
Copy link

@chad-codes chad-codes commented Oct 21, 2021

I noticed that this happens in the opts! macro already so I think it makes sense for histogram_opts! to also do this.

@chad-codes chad-codes changed the title feat: automatically convert Vec into HashMap feat: automatically convert Vec into HashMap in histogram_opts! macro Oct 21, 2021
@lucab
Copy link
Member

lucab commented Oct 25, 2021

Thanks for the PR! CI does not seem to like the proposed change, but I didn't look deeper into what's going on. On which toolchain version are you developing this? Does cargo test succeed for you locally?

@chad-codes
Copy link
Author

Whoops, I had made the original commit directly from Github without cloning and testing. I've updated the tests and everything is working.

This is a breaking change to existing functionality since histogram_opts! now expects labels to be passed in like labels! {"key" => "value"} instead of labels! {"key".to_string() => "value".to_string()}. This is how opts! works so I think it's an acceptable breaking change.

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