Add affected attribute format #175
Conversation
Following up on pypa#149 it seems like we have general agreement on what this format should be, so I've gone ahead and kicked off the PR 🎉 I took a liberty in how to deliniate two attributes (with a `;`). Happy to change that if there's disagreement on how to delimit multiple different attributes on the same osv payload. The osv payload is explicitly called out as equivalent to the dot-colon single line format as well. I also added a brief section linking to the osv schema.
README.md
Outdated
| which is equivalent to `PIL:ImageFont`. If a second attribute `ImageFont2` is also affected then a second payload should be added and delimited with a `;`. Eg. | ||
| ``` | ||
| { | ||
| attribute: "ImageFont", | ||
| modules: ["PIL"]; | ||
| attribute: "ImageFont2", | ||
| modules: ["PIL"] | ||
| } | ||
| ``` |
There was a problem hiding this comment.
0.02c: I don't think we should invent custom syntax at the serialization level here. If our goal is to express the dot product, maybe both can just be arrays?
{
"attributes": ["ImageFont", "ImageFont2"],
"modules": ["PIL"]
}There was a problem hiding this comment.
(This might have negative consequences I haven't thought of yet.)
Another option would be to have this represented as an array of these objects, e.g.:
[
{ "attribute": "ImageFont", "modules": ["PIL"] },
{ "attribute": "ImageFont2", "modules": ["PIL"] }
]There was a problem hiding this comment.
I should have probably used a better example here, but the idea is that the two should be completely independent (code, paths, etc...) aside from the top level module. I prefer option two of your suggestions for this, but @oliverchang what do you think best fits the osv style?
There was a problem hiding this comment.
+1 to not inventing custom syntax. We should keep this as plain JSON.
[
{ "attribute": "ImageFont", "modules": ["PIL"] },
{ "attribute": "ImageFont2", "modules": ["PIL"] }
]
as @woodruffw suggested seems pretty good to me!
Add json syntax for the markdown codeblock Co-authored-by: William Woodruff <[email protected]>
Add json syntax Co-authored-by: William Woodruff <[email protected]>
README.md
Outdated
| which is equivalent to `PIL:ImageFont`. If a second attribute `ImageFont2` is also affected then a second payload should be added and delimited with a `;`. Eg. | ||
| ``` | ||
| { | ||
| attribute: "ImageFont", | ||
| modules: ["PIL"]; | ||
| attribute: "ImageFont2", | ||
| modules: ["PIL"] | ||
| } | ||
| ``` |
There was a problem hiding this comment.
+1 to not inventing custom syntax. We should keep this as plain JSON.
[
{ "attribute": "ImageFont", "modules": ["PIL"] },
{ "attribute": "ImageFont2", "modules": ["PIL"] }
]
as @woodruffw suggested seems pretty good to me!
…on examples a little more explicit
|
Pushed up some changes based on the feedback and I made the json examples more explicit as well. Any other thoughts/spot any errors I made 👀 ? |
There was a problem hiding this comment.
This looks great to me with some minor changes!!
@woodruffw @sethmlarson WDYT?
There was a problem hiding this comment.
LGTM as well, modulo @oliverchang's suggestions.
Thanks @darakian!
Co-authored-by: Oliver Chang <[email protected]>
Co-authored-by: Oliver Chang <[email protected]>
|
Accepted both suggestions. 👍 |
Following up on #149 it seems like we have general agreement on what this format should be, so I've gone ahead and kicked off the PR 🎉 I took a liberty in how to deliniate two attributes (with a
;). Happy to change that if there's disagreement on how to delimit multiple different attributes on the same osv payload. The osv payload is explicitly called out as equivalent to the dot-colon single line format as well.I also added a brief section linking to the osv schema.I did in a prior version of the commit then removed it after noticing there was already a link to the osv schema 🤦Happy to move the target of this change or to change the wording or whatever :)