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

use relative path syntax #913

Merged
merged 1 commit into from
Nov 15, 2024

Conversation

ilans
Copy link
Collaborator

@ilans ilans commented Nov 14, 2024

Require filenames to use the relative path syntax.

To fix #910

@ilans ilans requested review from goneall, bact and zvr November 14, 2024 02:14
@ilans ilans added the Profile:Core Core Profile and related matters label Nov 14, 2024
@bact
Copy link
Collaborator

bact commented Nov 14, 2024

To remove the entire RFC is a big change, considering this one already passed the OMG review. At this stage, I understand that the change should be minimal and as necessary.

And the syntax for filename in RFC is more than just the prefix.

@zvr
Copy link
Member

zvr commented Nov 14, 2024

Sorry, @bact, I must be misunderstanding something.

How is any of the RFC relevant? The RFC talks about URIs, this field accepts filenames.

@goneall
Copy link
Member

goneall commented Nov 14, 2024

Agree with @zvr and @ilans - we should remove this reference.

@zvr @ramartin - What will this do to the ISO / OMG review if we merge this in?

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.

Not sure why URI syntax is relevant to file name syntax, not sure why it was included initially.

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.

Agree with the change - pending feedback from @zvr and @bobmartin3000 on impact to OMG review process

@goneall goneall added this to the 3.0.1 milestone Nov 14, 2024
@bact
Copy link
Collaborator

bact commented Nov 14, 2024

I'm fine to have the change. My concern is this is clearly not a change in a category of typo.

The URI can be a filename, so there is no harm keeping it.

But there can be a potential harm of prolonging the OMG review process.
If that's not the case, I don't see problem to have this change go to 3.0.1.

We defer a lot of issues and PRs in the past few months because of that OMG review reason.
I'm just try to keep it consistent that way. This is my understanding.

@bact
Copy link
Collaborator

bact commented Nov 14, 2024

This is Bob username @bobmartin3000

Copy link
Collaborator

@bact bact left a comment

Choose a reason for hiding this comment

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

If the process allows this to be in 3.0.1, please proceed.

@bobmartin3000
Copy link

Functionally we aren't changing anything with the removal of the reference to the RFC. I'm okay with the change but it is stretching the editorial change label.

Copy link

@bobmartin3000 bobmartin3000 left a comment

Choose a reason for hiding this comment

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

LGTM.

@kestewart kestewart merged commit 12c50ea into spdx:main Nov 15, 2024
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Profile:Core Core Profile and related matters
Projects
None yet
Development

Successfully merging this pull request may close these issues.

packageVerificationCodeExcludedFile filename prefix
6 participants