-
Notifications
You must be signed in to change notification settings - Fork 1
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
Add custom metadata to /packit/metadata endpoint #69
Add custom metadata to /packit/metadata endpoint #69
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. A minor suggested cleanup about the test, up to you if you want to change it or keep it as is to keep consistency with the rest of the file. I'm pretty ambivalent about it.
Did GitHub just forget about the code review comment I had written or something?? Sorry I will write it again |
.as_str() | ||
.unwrap(), | ||
"Projected Coverage for routine immunisation in PINE countries" | ||
); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I know the rest of the file is written in this style and you are just following that, but that is way more verbose than it needs to be.
assert_eq!(entries[0]["custom"]["orderly"]["artefacts"][3]["description"], "Projected Coverage ...");
serde_json::Value
implements the Index trait for both integers and string keys. If the value isn't of the right kind, the indexing returns a Null variant and the equality assertion would eventually fail. It also has a PartialEq<&str>
implementation, which means that you shouldn't need the final .as_str().unwrap()
call.
Alternatively, there is also the .pointer
method, eg.
entries[0].pointer("/custom/orderly/artefacts/3/description")
Pointer syntax comes from https://www.rfc-editor.org/rfc/rfc6901
https://docs.rs/serde_json/latest/serde_json/value/enum.Value.html#impl-Index%3CI%3E-for-Value
https://docs.rs/serde_json/latest/serde_json/value/enum.Value.html#method.pointer
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh, I hadn't seen pointer syntax before, that's handy!
I will use the index syntax.
This brings the implementation of the API to be more in line with the spec as it was in the README: namely, the /packit/metadata endpoint will now pass on the 'custom' property as part of the response. (The README also didn't show that the 'time' property was being returned.)