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

update security suppliedBy.md range to point to core suppliedby.md #488

Merged
merged 3 commits into from
Dec 11, 2023

Conversation

jeff-schutt
Copy link
Collaborator

3rd change to resolve #436

Intent is to have the suppliedBy property in the security profile reuse the suppliedBy property defined in the Core profile to avoid duplication of definitions.

@jeff-schutt
Copy link
Collaborator Author

In #436 I see Gary made a different suggestion here. Review requested to determine what change is needed.

@goneall
Copy link
Member

goneall commented Aug 29, 2023

I don't think we can have a duplicate suppliedBy in a different namespace. I think the approach we should use is to just reference the core namespace where we referenced the supplied by property per my suggestion #436 (comment).

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

@tsteenbe
Copy link
Member

tsteenbe commented Sep 6, 2023

Discussed in the Defects meeting of Sept 6th, 2023 by @goneall @jeff-schutt @rnjudge @tsteenbe - MarkDown encoding for this pull request is now correct but spec parser needs to be updated by @goneall before this can be merged see spdx/spec-parser#68.

@goneall
Copy link
Member

goneall commented Sep 6, 2023

Turns out I was wrong on the analysis of the CI failure - this is failing due to the Jinja changes - not due to handling /Core.

I tested this change locally on my machine with an earlier version of the spec-parser and it works, so this is safe to merge.

@goneall
Copy link
Member

goneall commented Sep 6, 2023

Just a note: this is a fix for the CI failure: spdx/spec-parser#69

@rnjudge
Copy link
Collaborator

rnjudge commented Nov 28, 2023

LGTM

@kestewart kestewart requested a review from rnjudge November 28, 2023 18:16
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.

Reviewed on call. Looks good.

@goneall
Copy link
Member

goneall commented Nov 28, 2023

To be merged once we merge the new parser - cc: @zvr

@zvr zvr assigned zvr and unassigned goneall Nov 29, 2023
@goneall
Copy link
Member

goneall commented Dec 11, 2023

The parser has been updated - merging...

@goneall goneall merged commit 7fa441d into main Dec 11, 2023
@goneall goneall deleted the update-security-suppliedBy branch December 11, 2023 23:37
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.

Move originatedBy and suppliedBy properties back to Element
6 participants