Skip to content

Conversation

rubensworks
Copy link
Member

This removes RDF-star parsing and instead implements for triple terms and reified triples from RDF 1.2.
I just wanted to raise this PR already to avoid duplicate efforts.

Note that some spec tests fail, because not all of them have been updated yet: w3c/rdf-tests#161

Also note that this PR does not update the writer yet.

This PR builds on top of #484 and should only be merged after w3c/rdf-star-wg#141 is resolved.

Copy link
Contributor

coderabbitai bot commented Jan 24, 2025

Important

Review skipped

Draft detected.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

✨ Finishing touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@jeswr
Copy link
Contributor

jeswr commented Jan 24, 2025

This removes RDF-star parsing and instead implements for triple terms

Note this repo now uses semantic release, and this is a breaking change. Make sure to include BREAKING CHANGE in the commit title and body when squash-merging this work into the main brach.

@RubenVerborgh
Copy link
Member

this is a breaking change

Fingers crossed that the RDF WG sees it that way indeed 😉

Excellent work, @rubensworks.

@RubenVerborgh RubenVerborgh force-pushed the main branch 3 times, most recently from 3a8218a to 64a6ce0 Compare April 12, 2025 20:03
@rubensworks
Copy link
Member Author

rubensworks commented May 8, 2025

@RubenVerborgh FYI, this now includes parsing of the new version directive (w3c/rdf-star-wg#141).
I wouldn't recommend merging yet though, as it's still being incorporated into the specs atm.

@rubensworks
Copy link
Member Author

@RubenVerborgh The new VERSION directive seems to be quite stable now across all specs.
How do you prefer going forward with this one?

We could merge and release as soon as the specs either go to CR or Rec status.
Or we could already merge now, and publish a prerelease.
(The latter option as first step would make Comunica dev around SPARQL 1.2 a lot easier)
Given the removed (=modified) RDF-star support, this should be a major version bump IMO.

Copy link
Member

@RubenVerborgh RubenVerborgh left a comment

Choose a reason for hiding this comment

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

@rubensworks All good, thanks a lot. This has been an impactful pull request.

Minor comments. Open question whether we also want to check (in the future) whether certain constructs are indeed version-allowed, like direction.

I'll leave the further handling to you, you can merge and publish as desired.

@rubensworks
Copy link
Member Author

Thanks for the review @RubenVerborgh.
After processing the comments, I will publish a prerelease asap (without merging to master for now).
Once the RDF specs go to CR or Rec, I will merge to master and do an actual release.

@rubensworks
Copy link
Member Author

For reference, the contents of this branch are now published as 2.0.0-beta.1 under the next tag.

@RubenVerborgh
Copy link
Member

All good, comments were minor; do proceed.

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