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

chore: deprecate shorthand properties #841

Conversation

kzantow
Copy link

@kzantow kzantow commented Mar 1, 2023

This PR adds deprecation to the "shorthand" properties in the schema and marks previously deprecated fields as such with the deprecated property.

Based on the SPDX WG meeting 2023/02/28, these fields will not be included in SPDX 3.0 and we want to indicate they are deprecated in the latest SPDX 2.3 JSON schema.

cc: @goneall @lumjjb

Signed-off-by: Keith Zantow <kzantow@gmail.com>
Copy link

@lumjjb lumjjb left a comment

Choose a reason for hiding this comment

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

lgtm. minor nits

"description" : "Indicates that a particular file belongs to a package.",
"description" : "DEPRECATED: use relationships instead of this field. Indicates that a particular file belongs to a package.",
"deprecated": true,
"$comment": "This field has been deprecated as it is a duplicate of using SPDXRef-<package-id> CONTAINS SPDXRef-<file-id> relationships",
Copy link

Choose a reason for hiding this comment

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

nit: for consistency, maybe just SPDXRef-PACKAGE CONTAINS?

Copy link
Author

Choose a reason for hiding this comment

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

If you're referring to the SPDXRef-DOCUMENT for the other deprecation text, I don't think there is a consistency problem because that is the actual value, whereas the SPDXRef-<package-id> needs to match the containing package, but I'm happy to change this if people feel strongly. I'll see if I can find a better way to identify this.

Copy link
Author

Choose a reason for hiding this comment

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

I did update the text here a bit, hopefully it's clear enough.

Signed-off-by: Keith Zantow <kzantow@gmail.com>
Signed-off-by: Keith Zantow <kzantow@gmail.com>
Copy link
Member

@goneall goneall left a comment

Choose a reason for hiding this comment

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

LGTM - @lumjjb if it looks good to you, I'll update the tools to generate the same changes and merge in this PR.

schemas/spdx-schema.json Show resolved Hide resolved
@goneall
Copy link
Member

goneall commented Mar 10, 2023

ping @lumjjb - does the latest changes look good? If so, I'll merge and update the tools

@lumjjb
Copy link

lumjjb commented Apr 12, 2023

yea LGTM!

@goneall
Copy link
Member

goneall commented Apr 12, 2023

I'll go ahead and merged.

I opened spdx/spdx-java-jackson-store#54 to track updating the Java tools.

@armintaenzertng do we need to open a similar issue for the Python tools?

@lumjjb @kzantow I'll assume you have the GoLang tools covered.

@goneall goneall merged commit 844144b into spdx:development/v2.3.1 Apr 12, 2023
@armintaenzertng
Copy link

armintaenzertng commented Apr 13, 2023

Thanks for the heads-up, Gary, but the python tools should be fine as we only support these fields during parsing, which we will most likely continue to do.

Edit: I just realized that we actually do output these fields; I'll remove that "feature".

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.

None yet

4 participants