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

Basic prep fix 1 #1801

Open
wants to merge 6 commits into
base: main
Choose a base branch
from
Open

Basic prep fix 1 #1801

wants to merge 6 commits into from

Conversation

bourgeoa
Copy link
Member

No description provided.

CxRes and others added 6 commits November 8, 2024 06:31
Using XML Date format instead of Internet Date format for Solid-PREP Notifications.
Swapped the `object` and `target`/`origin` properties in case of `Add`/`Delete` notifications respectively.
+ Notifications are identified by UUIDs.
+ Turtle notification template has the same fields as JSON-LD.
+ `notify:state` transmits Event-ID (which uniquely identifies a resource's state) and not E-Tag (which identify representations of the resource).
+ Parent reuses the generated Event-ID.
+ Event-ID is generated in PREP notification middleware itself.
+ No longer setting `Event-ID` response header upon resource mutation, thus reverting the middlewares for unsafe methods to their original state, before PREP was introduced.
Change the CI node-version to use caret range to ensure that tests are run on Node.js v20.
Values for `as:object`, `as:target` and `as:origin` predicates in the Turtle PREP-Notification template are IRI's, so they must be wrapped in angle brackets.

Co-authored-by: Sarven Capadisli <[email protected]>
@bourgeoa
Copy link
Member Author

LGTM
and conformance tests runed on https://solidcommunity.net:8443

@CxRes
Copy link
Collaborator

CxRes commented Nov 12, 2024

Just waiting for @csarven to check the final commit.

Copy link
Member

@csarven csarven left a comment

Choose a reason for hiding this comment

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

I don't quite understand this PR given #1800 . I reviewed it already. What's different here?

@CxRes
Copy link
Collaborator

CxRes commented Nov 12, 2024

Nothing. @bourgeoa prefers it this way, as it allows him to run CTH with solidcommunity.net when the branch is in the repo. And then he merges that.

@bourgeoa
Copy link
Member Author

Yes but not only.
It is in general a bad idea to merge directly in main from an outside branch.
There may be issues and main may be polluted, then you may need to revert.

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