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

Added more const for external reference to external.go #188

Merged
merged 3 commits into from
Feb 9, 2023

Conversation

neilnaveen
Copy link
Contributor

Signed-off-by: Neil Naveen [email protected]

-added more const for external.go, from the PR spdx#153

Signed-off-by: Neil Naveen <[email protected]>
Copy link
Collaborator

@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.

Thanks @neilnaveen for opening this up!

spdx/v2/common/external.go Outdated Show resolved Hide resolved
Copy link
Collaborator

@kzantow kzantow left a comment

Choose a reason for hiding this comment

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

Hey @neilnaveen if you are going to add these constants (which seems good 👍), could you add all the constants defined in the external references categories and annex f types?

Also these need aliases in the spdx/model.go.

I'd second @lumjjb 's request to replace the hardcoded values with these constants where appropriate, too. I think it would make sense to be part of this PR.

spdx/v2/common/external.go Outdated Show resolved Hide resolved
Signed-off-by: Neil Naveen <[email protected]>
@neilnaveen
Copy link
Contributor Author

Hey @neilnaveen if you are going to add these constants (which seems good 👍), could you add all the constants defined in the external references categories and annex f types?

Also these need aliases in the spdx/model.go.

I'd second @lumjjb 's request to replace the hardcoded values with these constants where appropriate, too. I think it would make sense to be part of this PR.

I have added all constants from the annex f types, and I have put them in their respective categories.

@neilnaveen neilnaveen requested review from kzantow and lumjjb and removed request for kzantow and lumjjb February 7, 2023 21:05
Copy link
Collaborator

@kzantow kzantow left a comment

Choose a reason for hiding this comment

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

Hi @neilnaveen this PR stills seems to be missing a number of changes from the original PR: https://github.com/spdx/tools-golang/pull/153/files it also does not include the aliases in the spdx package, are you able to fix this?

@neilnaveen
Copy link
Contributor Author

Hi @neilnaveen this PR stills seems to be missing a number of changes from the original PR: https://github.com/spdx/tools-golang/pull/153/files it also does not include the aliases in the spdx package, are you able to fix this?

I have added aliases into spdx/model.go. Could you also tell me which changes I have not added in this PR that was in #153?

@neilnaveen neilnaveen requested a review from kzantow February 9, 2023 18:21
@kzantow kzantow merged commit cdce85b into spdx:main Feb 9, 2023
neilnaveen added a commit to neilnaveen/tools-golang that referenced this pull request Feb 13, 2023
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.

3 participants