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

Change declared and concluded licenses to relationships #448

Merged
merged 2 commits into from
Aug 1, 2023

Conversation

goneall
Copy link
Member

@goneall goneall commented Jul 29, 2023

Implements decision made on whether licenses should be a property or relationship on the 2023-05-02 tech call

Note that this introduces issues as documented in #254

This fixes the parsing errors #443

Also fixes #Signed-off-by: Gary O'Neall [email protected]

Implements decision made on whether licenses should be
a property or relationship on the 2023-05-02 tech call

Note that this introduces issues as documented in #254

This fixes the parsing errors #443

Also fixes #Signed-off-by: Gary O'Neall <[email protected]>
@goneall
Copy link
Member Author

goneall commented Jul 29, 2023

Per the minutes from 2023-05-02 @kestewart @tsteenbe and @swinslow are to review the pull request for changing from properties to relationships.

@goneall
Copy link
Member Author

goneall commented Jul 29, 2023

Note: I would like to merge this to fix the parser errors.

@@ -25,10 +25,12 @@ between a Package and a File, between two Packages, or between one SPDXDocument
- buildDependency: Every `to` Element is a build dependency of the `from` Element
- buildTool: Build tool used to build an Element. This may be used to describe the build tool of a Build instance
- coordinatedBy: (Security) Used to identify the vendor, researcher, or consumer agent performing coordination for a vulnerability
- concludedLicense: Identifies the license that that SPDX data creator has concluded as governing the software Package, File or Snippet.
Copy link
Member

Choose a reason for hiding this comment

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

... "governing the software Artifact."

Copy link
Contributor

Choose a reason for hiding this comment

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

Please be explicit, what is "to" and what is "from" here

- contains: Every `to` Element is contained by the `from` Element
- configOf: (Build) Configuration information applied to an Element instance during a LifeycleScopeType period. Example: Build configuration of the build instance
- copy: Every `to` Element is a copy of the `from` Element
- dataFile: Every `to` Element is a data file related to the the `from` Element
- declaredLicense: dentifies the license information actually found in the software Package, File or Snippet, for example as detected by use of automated tooling.
Copy link
Member

Choose a reason for hiding this comment

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

"software Artifact"

Copy link
Contributor

Choose a reason for hiding this comment

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

aslo please fix typo "dentifies" --> "Identifies", can you also be explict as to the "to" and "from". ie. what goes on which side of the relationship.

Copy link
Member Author

Choose a reason for hiding this comment

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

Updated per comments

Copy link
Contributor

@kestewart kestewart left a comment

Choose a reason for hiding this comment

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

Please be explicit about what is "to" and what should be "from" for concludedLicense & declaredLicense.

- contains: Every `to` Element is contained by the `from` Element
- configOf: (Build) Configuration information applied to an Element instance during a LifeycleScopeType period. Example: Build configuration of the build instance
- copy: Every `to` Element is a copy of the `from` Element
- dataFile: Every `to` Element is a data file related to the the `from` Element
- declaredLicense: dentifies the license information actually found in the software Package, File or Snippet, for example as detected by use of automated tooling.
Copy link
Contributor

Choose a reason for hiding this comment

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

aslo please fix typo "dentifies" --> "Identifies", can you also be explict as to the "to" and "from". ie. what goes on which side of the relationship.

@@ -25,10 +25,12 @@ between a Package and a File, between two Packages, or between one SPDXDocument
- buildDependency: Every `to` Element is a build dependency of the `from` Element
- buildTool: Build tool used to build an Element. This may be used to describe the build tool of a Build instance
- coordinatedBy: (Security) Used to identify the vendor, researcher, or consumer agent performing coordination for a vulnerability
- concludedLicense: Identifies the license that that SPDX data creator has concluded as governing the software Package, File or Snippet.
Copy link
Contributor

Choose a reason for hiding this comment

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

Please be explicit, what is "to" and what is "from" here

Copy link
Contributor

@kestewart kestewart left a comment

Choose a reason for hiding this comment

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

Thanks. That fixes the issue note.

@goneall
Copy link
Member Author

goneall commented Aug 1, 2023

Since @zvr is away from the office this week and I have made his requested changes plus we have a positive review from @kestewart , I'm going to merge this. It should fix the error's we're seeing in the CI.

If anyone has any additional improvements on this, please open a new PR.

@goneall goneall merged commit fc9f506 into main Aug 1, 2023
1 check passed
@goneall goneall deleted the licenseRelationship branch August 1, 2023 00:22
@goneall goneall mentioned this pull request Aug 4, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants