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 a comment in the examples/ folder for each example program explaining how to run it #115 #120

Closed
wants to merge 2 commits into from

Conversation

CatalinStratu
Copy link
Contributor

Signed-off-by: CatalinStratu <catalinstratu45@gmail.com>
Signed-off-by: CatalinStratu <catalinstratu45@gmail.com>
@CatalinStratu
Copy link
Contributor Author

Hi @swinslow , what do you think about this PR?

@swinslow
Copy link
Member

Hi @CatalinStratu, thanks for this! I was originally thinking that the PR for #115 would just be adding comments listing what parameters to pass. But I like what you're doing here, by actually pointing to specific sample SPDX documents, etc. in the repo so that users really can just run the example commands as-is.

I would suggest making a few changes:

  • The testdata/ directory is really meant to be where samples are stored for the automated test suites in the rest of the tools-golang packages. I like that you're adding some sample SPDX files to load and analyze, but if they aren't used by the main test suites and are just for the examples, then I would put them under the examples/ directory -- e.g. maybe in a new subfolder examples/sample-docs or something like that.
  • For the contents of that examples/sample-docs folder, the best samples to use would be those from the main spdx-spec repo. The JSON, RDF and tag-value files from https://github.com/spdx/spdx-spec/tree/development/v2.2.2/examples would be great to include here, rather than within the testdata/` folders.
  • For the additional examples from https://github.com/spdx/spdx-examples, you can also include some of the SPDX documents from that repo if you'd like. However, I'd suggest only including the SPDX documents themselves, not the other parts of the repo. In other words, from this commit it looks like you're adding other files like READMEs, sample code, etc. -- I would leave all of that out and just add the .spdx documents, since they're all that the example code will be using.

If you can make those changes and then re-submit the updated version, that would be great. I'd suggest either squashing your commits or else submitting as a new PR, rather than merging all of the commits currently in this PR (again, so that we aren't ending up with the extra spdx-examples files in the commit tree here).

Thanks again! And let me know if any of my comments aren't clear.

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.

None yet

2 participants