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