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

fix: some broken IETF links #2798

Merged
merged 1 commit into from
Dec 23, 2021
Merged

fix: some broken IETF links #2798

merged 1 commit into from
Dec 23, 2021

Conversation

saibotsivad
Copy link
Contributor

From #2796 some IETF links were correct in the markdown syntax, but the generated HTML had broken links.

For example, the markdown for the Request Body Object > Fixed Fields (4.8.13.1) looks like this:

The key is a media type or [media type range](https://tools.ietf.org/html/rfc7231#appendix-D) and

But the rendered HTML looks like this:

The key is a media type or [media type range]appendix-D) and

The problem occurs due to the markdown pre-processor code used to adjust RFC links.

It looks like the original intent of lines 267-269 was to transform links that looked like this:

[RFC2119](https://tools.ietf.org/html/rfc2119)

Into this format:

[[!RFC2119]]

I'm not sure what the reasoning was for that, but if I simply delete lines 267-269, it does resolve the issue.

To be thorough, I inspected the output for instances of lines containing https://tools.ietf.org/html/rfc at the end, aka what lines 267-269 would have operated on, and it's all links where the text is not RFC\d+. Specifically:

  • In versions 3.0.0-3.1.0 the "Media Type Object" has two instances of <a href="https://tools.ietf.org/html/rfc7231#appendix-D">media type range</a>
  • In versions 3.0.0-3.1.0 the "Runtime Expressions" section has <a href="https://tools.ietf.org/html/rfc5234">ABNF</a>
  • In version 3.0.0 only, in the "Operation Object" section under "Fixed Fields", the requestBodies section has one for each method, e.g. <a href="https://tools.ietf.org/html/rfc7231#section-4.3.1">GET</a>

Since that's the only instances affected by removing lines 267-269, this change seems very safe.

@saibotsivad
Copy link
Contributor Author

@webron any progress on this? Just wanting to make sure it didn't fall through the cracks. 😅

@webron
Copy link
Member

webron commented Dec 15, 2021

Thanks, @saibotsivad. It looks pretty straight forward but would like someone else from the TSC to look into it before the merging. I added it to this week's meeting agenda. I can't attend to it this week, but hopefully they'll be able to review it. If you can join the meeting, it could help with the review process but not required.

@saibotsivad
Copy link
Contributor Author

Neat! I'll see if I can make it to the meeting. 👍

@darrelmiller darrelmiller merged commit 115cacc into OAI:main Dec 23, 2021
@MikeRalphson MikeRalphson self-assigned this Mar 13, 2023
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