Skip to content

Update text and examples regarding the fetching of TDs #524

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

Merged
merged 8 commits into from
Jan 15, 2024

Conversation

JKRhb
Copy link
Member

@JKRhb JKRhb commented Nov 30, 2023

In the current Editor's draft, there are a couple of leftovers from previous versions where the requestThingDescription had not been introduced yet. This PR updates the text, incorporates the latest changes, and also removes an outdated appendix that explains why there is no fetch method anymore (which has now de facto been reintroduced). Maybe some text regarding the validation of TDs could be added/moved elsewhere, though.


Preview | Diff

Copy link
Contributor

@danielpeintner danielpeintner left a comment

Choose a reason for hiding this comment

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

Content-wise LGTM

see comment below about quotes...

@relu91
Copy link
Member

relu91 commented Dec 4, 2023

Call 04/12/2023:

  • @zolkis don't agree to remove fetch from the examples. It is still valid, since the convenience function does not cover all the use cases of WoT, which might need to use e.g. HTTP options. We still have to mention it and we need to explain that it is possible to fetch, and make examples to highlight when you need fetch and the convenience function.
  • @danielpeintner best practice is to use the new method because they get some validation.
  • @JKRhb fetch and HTTP requests are still mentioned in the text, it could cover the comments above.

Group decision: rework the rationale removed by mentioning the use cases for fetch and requestThingDescription. Double check mentioning of fetch or HTTP requesting in the whole spec text.

Copy link
Contributor

@zolkis zolkis left a comment

Choose a reason for hiding this comment

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

IMHO we need to keep the (reworked) fetch and validate section.

@zolkis
Copy link
Contributor

zolkis commented Dec 4, 2023

Some relevant historical discussions, please check:
#78
#162
#216

Copy link
Member

@relu91 relu91 left a comment

Choose a reason for hiding this comment

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

I think it is better to change it into an assertion (?)

@relu91
Copy link
Member

relu91 commented Dec 11, 2023

Call 11/12

  • @ashimura Having the appendix is fine but we should explain its purpose
  • @zolkis we can introduce it in the section about requestThingDescription
  • @danielpeintner matching spec sections with rationale sections makes sense. I would do this in general (we should have already probably worth checking)

Copy link
Contributor

@zolkis zolkis left a comment

Choose a reason for hiding this comment

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

From my part, we can deal with improving the section 5 vs appendix A in a new issue and merge this PR.

@relu91
Copy link
Member

relu91 commented Jan 15, 2024

Call 15/01:

@relu91 relu91 merged commit d0c5f8c into w3c:main Jan 15, 2024
@relu91 relu91 mentioned this pull request Jan 15, 2024
@JKRhb JKRhb deleted the request-td branch January 16, 2024 10:01
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.

4 participants