Skip to content
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

Document ADR 0008 about unrecognized fields #1343

Merged
merged 1 commit into from
Apr 16, 2021

Conversation

MVrachev
Copy link
Collaborator

@MVrachev MVrachev commented Apr 8, 2021

Please fill in the fields below to submit a pull request. The more information
that is provided, the better.

Fixes #1266

Description of the changes being introduced by the pull request:
Even though, this ADR documents something already implied in the TUF
spec in document formats
it seems better to document this decision clearly so that it could be
referenced and give an explanation why someone can load a metadata file
with additional unrecognized fields.

Signed-off-by: Martin Vrachev mvrachev@vmware.com

Please verify and check that the pull request fulfills the following
requirements
:

  • The code follows the Code Style Guidelines
  • Tests have been added for the bug fix or new feature
  • Docs have been added for the bug fix or new feature

Copy link
Contributor

@mnm678 mnm678 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks great, thanks @MVrachev! This should help us move forward with some of the TAPs

Copy link
Member

@joshuagl joshuagl left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for capturing this @MVrachev. I made a few minor comments, otherwise this looks good.

docs/adr/0008-accept-unrecognised-fields.md Outdated Show resolved Hide resolved
docs/adr/0008-accept-unrecognised-fields.md Outdated Show resolved Hide resolved
docs/adr/0008-accept-unrecognised-fields.md Outdated Show resolved Hide resolved
@MVrachev
Copy link
Collaborator Author

MVrachev commented Apr 9, 2021

@joshuagl I addressed your comments and amend the commit so it can be ready to be merged.

PS: I had to force-push a couple of times to fix some formatting issues.

@MVrachev MVrachev force-pushed the adr8 branch 2 times, most recently from 73fa130 to 6a68dfb Compare April 9, 2021 11:40
Even though, this ADR documents something already implied in the TUF
spec in [document formats](https://theupdateframework.github.io/specification/latest/#document-formats)
it seems better to document this decision clearly so that it could be
referenced and give an explanation why someone can load a metadata file
with additional unrecognized fields.

Signed-off-by: Martin Vrachev <mvrachev@vmware.com>
@MVrachev
Copy link
Collaborator Author

I am not sure why, but we have a CI failure again on Windows
https://github.com/theupdateframework/tuf/pull/1343/checks?check_run_id=2342301986
@jku does this look familiar to other Windows CI failures?

@jku
Copy link
Member

jku commented Apr 14, 2021

Not at all, this is tox calling pip and pip failing to download "typed-ast" in some fantastic way...

@jku
Copy link
Member

jku commented Apr 16, 2021

For reference: the CI failure seems like a pypi problem (made worse by our decision to use pre-release build tooling so downloading more often). Disappeared on re-run, I'm not filing any issues on that. Merging this.

@jku jku merged commit ed3d00e into theupdateframework:develop Apr 16, 2021
@MVrachev MVrachev deleted the adr8 branch April 16, 2021 10:21
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.

Should we accept metadata that includes unrecognised fields?
4 participants