Skip to content

feat: automatically convert Vec into HashMap in histogram_opts! macro - #422

Closed
chad-codes wants to merge 2 commits into
tikv:masterfrom
chad-codes:patch-1
Closed

chad-codes wants to merge 2 commits into
tikv:masterfrom
chad-codes:patch-1

Conversation

@chad-codes

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

Copy link
Copy Markdown

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
Signed-off-by: Chad Greenburg <cgreenburg93@gmail.com>
@lucab

lucab commented Oct 25, 2021

Copy link
Copy Markdown
Member

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?

Signed-off-by: Chad Greenburg <cgreenburg93@gmail.com>
@chad-codes

Copy link
Copy Markdown
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.

@chad-codes chad-codes closed this Nov 14, 2025
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