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.

Sign up for GitHub

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

Jump to bottom

Add HTTP section #26

Merged
merged 7 commits into from
Aug 29, 2019
Merged

Add HTTP section #26

merged 7 commits into from
Aug 29, 2019

Conversation

RubenVerborgh
Copy link
Contributor

Issues such as inrupt/wac-ldp#114 indicate that we want to explicitly refer to the HTTP specification.

I am following the intent of the current draft specs and implementations, with the exception of:

Feedback most welcome.

@RubenVerborgh RubenVerborgh self-assigned this Aug 6, 2019
@dmitrizagidulin
Copy link
Member

(Big thumbs up for this PR in general, I think this is an important section.)

kjetilk
kjetilk requested changes Aug 7, 2019
View reviewed changes
Copy link
Member

@kjetilk kjetilk left a comment

Choose a reason for hiding this comment

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

I have a few inline comments, other than that, it looks good.

main/resource-access.bs Outdated Show resolved Hide resolved
main/resource-access.bs Show resolved Hide resolved
main/resource-access.bs Outdated Show resolved Hide resolved
TallTed
TallTed reviewed Aug 7, 2019
View reviewed changes
main/resource-access.bs Outdated Show resolved Hide resolved
@kjetilk kjetilk mentioned this pull request Aug 7, 2019
Open
kjetilk
kjetilk approved these changes Aug 7, 2019
View reviewed changes
Copy link
Member

@kjetilk kjetilk left a comment

Choose a reason for hiding this comment

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

OK, sounds good. I added #27 to address the upgrade mechanism.

@kjetilk kjetilk mentioned this pull request Aug 7, 2019
Open
acoburn
acoburn approved these changes Aug 7, 2019
View reviewed changes
Copy link
Member

@acoburn acoburn left a comment

Choose a reason for hiding this comment

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

👍

Setting range request support at MAY makes a lot of sense to me (Trellis supports range requests for LDP-NR resources only)

csarven
csarven approved these changes Aug 8, 2019
View reviewed changes
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.

Looks good!

main/resource-access.bs Show resolved Hide resolved
main/resource-access.bs Show resolved Hide resolved
justinwb
justinwb reviewed Aug 9, 2019
View reviewed changes
main/security.bs Outdated Show resolved Hide resolved
justinwb
justinwb approved these changes Aug 9, 2019
View reviewed changes
Copy link
Member

@justinwb justinwb left a comment

Choose a reason for hiding this comment

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

Looking great 👍

@justinwb justinwb removed the request for review from TallTed August 9, 2019 21:22
@RubenVerborgh RubenVerborgh mentioned this pull request Aug 12, 2019
Closed
pmcb55
pmcb55 approved these changes Aug 12, 2019
View reviewed changes
Copy link

@pmcb55 pmcb55 left a comment

Choose a reason for hiding this comment

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

Looks great to me (with caveat on #14!).

@RubenVerborgh RubenVerborgh merged commit 62e89e9 into master Aug 29, 2019
@RubenVerborgh RubenVerborgh deleted the feature/http branch August 29, 2019 12:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@csarven csarven csarven approved these changes

@kjetilk kjetilk kjetilk approved these changes

@justinwb justinwb justinwb approved these changes

@dmitrizagidulin dmitrizagidulin dmitrizagidulin approved these changes

@pmcb55 pmcb55 pmcb55 approved these changes

@acoburn acoburn acoburn approved these changes

@TallTed TallTed TallTed approved these changes

@michielbdejong michielbdejong Awaiting requested review from michielbdejong

@timbl timbl Awaiting requested review from timbl

Assignees

@kjetilk kjetilk

@RubenVerborgh RubenVerborgh

Labels
category: editorial Concerns phrasing/wording doc: Protocol
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
8 participants
@RubenVerborgh @dmitrizagidulin @kjetilk @csarven @justinwb @pmcb55 @acoburn @TallTed