Add deprecated node mapping and info/example to readme #31
Conversation
README.md
Outdated
| @@ -81,6 +81,36 @@ Entries that are nested within another Entry. Only Categories or Sub-Categories | |||
| } | |||
| ``` | |||
|
|
|||
| ### Deprecated node mapping | |||
| When breaking changes such as deletion/collapsing of IDs or moving to a different category occur, the `deprecated_node_mapping` will serve as a reference to find the latest ids for deprecated nodes. | |||
There was a problem hiding this comment.
Can we replace category with parent since the subcategory can change as well.
There was a problem hiding this comment.
Thoughts on adding mapped like so: to find the latest mapped ids?
README.md
Outdated
| ### Deprecated node mapping | ||
| When breaking changes such as deletion/collapsing of IDs or moving to a different category occur, the `deprecated_node_mapping` will serve as a reference to find the latest ids for deprecated nodes. | ||
|
|
||
| **Any time such a breaking change occurs, it requires a new entry to be added to the deprecated_node_mapping** |
There was a problem hiding this comment.
can we use hyphens for the name so it matches the file name:
deprecated-node-mapping.json
README.md
Outdated
| "2.1": "unvalidated_redirects_and_forwards.open_redirect.get_based" | ||
| }, | ||
| "unvalidated_redirects_and_forwards.open_redirect.get_based_unauthenticated": { | ||
| "2.1": "unvalidated_redirects_and_forwards.open_redirect.get_based" |
There was a problem hiding this comment.
we can probably just use a single example for brevity
There was a problem hiding this comment.
up to you, but I think the full example helps describe how multiple nodes are being collapsed into one.
| #### Example | ||
| _2 nodes being collapsed into 1_ | ||
| ```json | ||
| { |
There was a problem hiding this comment.
Maybe move this concrete example into contributing as well?
There was a problem hiding this comment.
In CONTRIBUTING.md we only have examples with formats so far. I think that's fine, unless we want to add concrete examples for both.
|
What do you guys want to do when we expand one entry into two like in #34? |
|
@plr0man I am not sure we create a mapping if the previous node gains specificity. While that will create multiple nodes names that partially overlap in an aggregate, I think that is ok. |
|
@barnett before you merge this I would like to write a little validator script that basically checks that the mapped nodes exist in the specified versions. |
Example error output:
Errors: [{'missing_node': u'unvalidated_redirects_and_forwards.open_redirect.get_basedd', 'version': u'1.2'}]
adamrdavid
force-pushed
the
add-deprecated-node-mapping
branch
from
d2c15aa
to
1f701c1
Compare
adamrdavid
dismissed
barnett’s stale review
made changes afterwards, would like another review, specifically around the validator
|
Make sure to exit with Unix exit code so that Docker understands what's going on |
|
Maybe grab @danielhtrauner 's eyes on that 🌶 🌶 🐍 code |
|
Probably better to use an actual |
|
@danielhtrauner good idea will do ;) |
adamrdavid
force-pushed
the
add-deprecated-node-mapping
branch
from
a81c524
to
5f4276e
Compare
closes this issue: #24