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

add(package): add const for external reference #153

Closed

Conversation

masahiro331
Copy link

When using this library, we want to use the values defined in the specification.
It would be nice if publish these consts as an official library.

Signed-off-by: masahiro331 <m_fujimura@r.recruit.co.jp>
@masahiro331 masahiro331 force-pushed the feat/add_const_for_package branch from 4561323 to 5d97108 Compare July 23, 2022 13:30
@swinslow
Copy link
Member

Hi @masahiro331, thanks for this!

I'm generally agreed with adding more constants like this. That said, #146 is going to be making extensive changes to the repo, including this file. I'm expecting to merge #146 by next Wednesday if there aren't any objections.

So after that PR is merged, it will likely make sense to then add these constants in the new spdx/common/ directory -- maybe in a new file like spdx/common/constants.go or something like that.

I'll leave this open so we can track it, but I think I'd suggest doing it in that order so that this one doesn't create a new conflict for #146. Thanks!

@lumjjb
Copy link
Collaborator

lumjjb commented Aug 18, 2022

@masahiro331 PR #146 has been merged now... Can you help rebase the code.. I think this should now be part of:

https://github.com/spdx/tools-golang/tree/main/spdx/common

@lumjjb lumjjb added this to the 0.4.0 milestone Aug 26, 2022
@masahiro331
Copy link
Author

Thank you, I have just started this task.

@lumjjb
Copy link
Collaborator

lumjjb commented Sep 4, 2022

Awesome - i've assigned the issue to you! Thanks @masahiro331

@lumjjb
Copy link
Collaborator

lumjjb commented Sep 28, 2022

Hi @masahiro331 , We have a file for this now : https://github.com/spdx/tools-golang/blob/main/spdx/common/external.go. I'd recommend branching main again and applying your changes to this file!

@lumjjb
Copy link
Collaborator

lumjjb commented Oct 3, 2022

Related #163, still looks like the high level categories still need some constants i.e. "PACKAGE-MANAGER"

@kzantow
Copy link
Collaborator

kzantow commented Jan 18, 2023

Hi @masahiro331 -- there was a significant refactoring in this library, are you able to adjust this PR for the changes?

@neilnaveen
Copy link
Contributor

Hi @masahiro331 -- there was a significant refactoring in this library, are you able to adjust this PR for the changes?

I would like to work on this.

@neilnaveen
Copy link
Contributor

neilnaveen commented Feb 3, 2023

Should the file be moved back into the SPDX common folder?

@kzantow
Copy link
Collaborator

kzantow commented Feb 3, 2023

@neilnaveen I don't believe so -- these are already available with aliases at the top-level github.com/spdx/tools-golang/spdx package. If it turns out that these are identical or a subset of what V3 has, I'd say we could move it at that point.

neilnaveen added a commit to neilnaveen/tools-golang that referenced this pull request Feb 4, 2023
-added more const for external.go, from the PR spdx#153

Signed-off-by: Neil Naveen <42328488+neilnaveen@users.noreply.github.com>
@neilnaveen
Copy link
Contributor

Fixed in #188

@kzantow
Copy link
Collaborator

kzantow commented Feb 10, 2023

Hi @masahiro331 thank you for this contribution -- it looks like most of these constants were added as part of #188 so I'm going to close this for now but please reopen if you would like some additional changes!

@kzantow kzantow closed this Feb 10, 2023
@masahiro331 masahiro331 deleted the feat/add_const_for_package branch February 13, 2023 09:12
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.

5 participants