solid / notifications

Solid Notifications Technical Reports
https://solid.github.io/notifications/protocol
MIT License
11 stars 7 forks source link

Feat: update HTTP refs #187

Closed woutermont closed 9 months ago

woutermont commented 10 months ago

Following up on https://github.com/solid/specification/issues/471, this PR updates the references to HTTP RFCs from the obsolete 723x to the replacing 911x.

Changes

Commit Target Correction class Note
0b7091a #ResourceServer 2 (no conformance impact; no change in semantics of 'server') updates ref RFC7230/7231 => RFC9110/9112 in Resource Server product class definition
0b7091a #DiscoveryClient 2 (no conformance impact; no change in semantics of 'client') updates ref RFC7230/7231 => RFC9110/9112 in Discovery Client product class definition
0b7091a #SubscriptionClient 2 (no conformance impact; no change in semantics of 'client') updates ref RFC7230/7231 => RFC9110/9112 in Subscription Client product class definition
0b7091a #SubscriptionServer 2 (no conformance impact; no change in semantics of 'server') updates ref RFC7230/7231 => RFC9110/9112 in Subscription Server product class definition
bf8c471 #subscription-server-subscription-request-methods 2 (no conformance impact; no change in semantics of methods updates ref RFC7231 => RFC9110 in subscription server request methods
b7b9b0f #subscription-server-subscription-request-methods 2 (no conformance impact; grammatical fix) removes double appearance of 'method'
90056c4 #notify-accept 3 (negligible conformance impact: additional extension parameters are no longer allowed; see changelog [*] and relevant passage) updates ref RFC7231 => RFC9110 in definition of the 'accept' notification feature
34f239c #bib-rfc7231 1 (no content change, only bibliography) removes reference to obsolete RFC7231
56866f5 0ecbb3c #bib-rfc9110 1 (no content change, only bibliography) inserts reference to replacing RFC9110
56866f5 0ecbb3c #bib-rfc9112 1 (no content change, only bibliography) inserts reference to replacing RFC9112

HTML Preview | HTML Diff

If there still are any concerns about impact to conformance, you can check the relevant changelogs here:

On a sidenote: do you all manually update the HTML here? Bikeshed or ReSpec are quite handy in keeping track of references, amongst other things.

CxRes commented 10 months ago

@woutermont Thanks for your work dragging Solid specs into 2023!

Maybe consider dropping references to RFC9112 altogether. RFC9205 recommends:

Specifications should use [HTTP] as the primary reference for HTTP; it is not necessary to reference all of the specifications in the HTTP suite unless there are specific reasons to do so (e.g., a particular feature is called out).

woutermont commented 10 months ago

Interesting. Could you create a specific issue for that, @CxRes? (Preferably in solid/specification)

csarven commented 10 months ago

As I understand it, 9110 does not entirely cover 9112 (which is required in and itself). I would not object to only using 9110 if that's generally sufficient as far as completeness or conformance is concerned but would appreciate a better assessment on the situation so that we can make a decision.

CxRes commented 10 months ago

As far as SNP goes, I see no place where we require an explicit reference to HTTP/1.1 spec. This conformance requirement should only be coming from Solid spec instead.

woutermont commented 10 months ago

I agree with @CxRes that the notifications protocol just needs to rely on solid protocol plus specific parts of rfc9110 when needed. But within the scope of this PR, let's leave it at the update of the RFCs themselves.

woutermont commented 10 months ago

Given the lack of impact of this change, I suggest merging this by next meeting 2023-08-30, at the discretion of the authors, of course. @acoburn @csarven @elf-pavlik @CxRes

Also: should I include these changes in the changelog, or is that done when a new version is actually published?

csarven commented 10 months ago

Short answer: let's add in the entry "| 3 | Document | Change from HTTP 7230 and 7231 to HTTP 9110 and 9112 references |"

If the PR comment has sufficient information about the changes (e.g., as you have here), I have minor preference to add in the changelog item just before merging the PR - speed things up without having to re-update the changelog during the review process. But besides that, it is no big deal to do it after the PR merge either. Essentially when a new TR is published, the editors should process all commits (and elsewhere) to ensure they are captured in the changelog.

As for this particular PR, I don't have a strong opinion on listing this change as a separate item or have it simply be captured by the second item https://solid.github.io/notifications/protocol#d7f80478-0b63-4eee-a61c-59e8e659542e (I need to add ids to changelog ids):

Change Class Subject Description
2 Document Amend language and document details.

Class 3 changes should be listed, and I think this PR touches that at least with Accept.

csarven commented 10 months ago

This conformance requirement should only be coming from Solid spec instead.

I agree with @CxRes that the notifications protocol just needs to rely on solid protocol

I'm not sure if I'm misinterpreting the context but what you are saying may inaccurate since SNP doesn't require the whole of SP. We tried to keep it compatible so that an SP Server could be extended to potentially conform to SNP's ResourceServer, SubscriptionServer, NotificationReceiver/Sender. It was just the SP AuthN/Z bits we referenced.


@acoburn @CxRes @elf-pavlik , can you please also have a look / (re-)review? This PR should stick to the minimal - upgrade references, and keep functionality as is. Move to other issues/PRs for other changes as needed.

CxRes commented 10 months ago

The only place afaict the issue arises is wrt classes of products. I think we can drop RFC9112 references here without any issue.

In general though, there is a desire to have common classes of products across specs, possible in a separate document (see 30/08/2023 CG minutes) which will be discussed in a special meeting. There we can derive SNP products from SP products. It is only in that sense there is a SNP dependency on SP or some need for congruence between the two specs. Otherwise, I agree the two specs must be independent.

woutermont commented 10 months ago

@CxRes, could you also create a specific issue/PR for dropping refs to RFC9112 specifically? If you could then approve the current PR, we can progress step by step.

Also pinging @elf-pavlik and @acoburn again.

CxRes commented 10 months ago

I'll approve this. No need for a second issue - just refer to the one in Solid specification!