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

348 allow sbol closure semantics #351

Merged
merged 10 commits into from
Nov 12, 2021
Merged

Conversation

bbartley
Copy link
Contributor

@bbartley bbartley commented Nov 9, 2021

Include the property name from the SHACL report so a user knows which
property the message is about.
Add a simpler and self-contained test case to reproduce issue 48 for
a single property.
Fix up some inline comments and add links to issues.
Copy link
Collaborator

@tcmitchell tcmitchell left a comment

Choose a reason for hiding this comment

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

These changes all look good. I made some minor changes and a couple of additions:

  • Fixed up inline comments in your unit tests to include links to issues
  • Added a self-contained unit test that demonstrates the issue on a single property without the need of a 1,500 line file and 16 error messages. I retained your larger unit test as well.
  • Improved the messages generated from the SHACL report so they indicate to which property the issue relates.

@bbartley
Copy link
Contributor Author

Improved the messages generated from the SHACL report so they indicate to which property the issue relates.

Cool! Thanks for the review and the improvements.

@bbartley bbartley merged commit 6e897b8 into main Nov 12, 2021
@tcmitchell tcmitchell added this to the beta milestone Nov 12, 2021
@tcmitchell tcmitchell deleted the 348-allow_sbol_closure_semantics branch November 12, 2021 16:25
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.

Validation treats missing TopLevel objects as errors
2 participants