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

Bug fixes in Examples (PR #5 for issue #94) #112

Merged
merged 5 commits into from
Aug 17, 2023

Conversation

anatoly-scherbakov
Copy link
Contributor

@anatoly-scherbakov anatoly-scherbakov commented Jul 23, 2023

Why

YAML-LD samples have a few bugs.

What

This PR is fixing those.


Preview | Diff

@anatoly-scherbakov
Copy link
Contributor Author

@gkellogg please check this out when you have a moment 🙂

Copy link
Member

@gkellogg gkellogg left a comment

Choose a reason for hiding this comment

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

I suggested using relative links for yaml files. If there is a rendering issue, this might be addressed differently.

spec/index.html Outdated
@@ -342,7 +342,7 @@ <h2>Introduction</h2>

<div class="ednote">
See YAML-LD description of this spec at
<a href="https://github.com/json-ld/yaml-ld/blob/main/spec/spec.yaml">
<a href="https://github.com/json-ld/yaml-ld/blob/main/spec/data/spec.yaml" target="_blank">
Copy link
Member

Choose a reason for hiding this comment

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

It's cleaner to keep these references as relative to the spec, so when they're published, they show up properly. That would make it <a href="data/spec.yaml">...</a>.

@anatoly-scherbakov
Copy link
Contributor Author

Done so, — but it would seem it does not render. For instance: https://pr-preview.s3.amazonaws.com/json-ld/yaml-ld/pull/data/spec.yaml does not work.

@gkellogg
Copy link
Member

Done so, — but it would seem it does not render. For instance: https://pr-preview.s3.amazonaws.com/json-ld/yaml-ld/pull/data/spec.yaml does not work.

PR Preview doesn't have access to associated files, but you can see them via GitHack. This better reflects how the spec will render when merged.

@anatoly-scherbakov anatoly-scherbakov merged commit fc592d6 into main Aug 17, 2023
@anatoly-scherbakov anatoly-scherbakov deleted the issue-94-convenience-contexts-examples branch August 17, 2023 08:00
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.

2 participants