Skip to content
This repository was archived by the owner on Nov 2, 2023. It is now read-only.

Updating of examples / samples #215

Closed
wants to merge 12 commits into from
Closed

Updating of examples / samples #215

wants to merge 12 commits into from

Conversation

retrosight
Copy link

With this pull request it's my intent to...

  • Update the examples to Draft 7.
  • Create pointers back to the spec.
  • Pure markdown (no embedded HTML)
  • Generally improve the illustrations.

@handrews handrews requested a review from Relequestual June 5, 2018 21:46
@handrews
Copy link
Contributor

handrews commented Jun 6, 2018

Thanks so much, @retrosight! I will review this in detail soon. I'd also like to get @Relequestual's review as the lead for the web site.

@retrosight there is a test failing in Travis CI for one of the schema files (perhaps it was removed)- can you edit .travis.yml to fix/remove the test? The PR needs to pass CI before merging.

@Relequestual
Copy link
Member

Thanks for this! I'm likely to be able to review next week.

Copy link
Contributor

@philsturgeon philsturgeon left a comment

Choose a reason for hiding this comment

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

Give this man a medal and merge the PR!

@retrosight
Copy link
Author

Thanks for the vote of confidence, @philsturgeon. ;-) Let's hold off on merging a bit as I'm still working through all of the examples -- one still left to go: example2.md -- plus I'd like to invite the community over on Slack to walk through them and provide feedback.

@handrews I fixed the build fail.

@philsturgeon
Copy link
Contributor

philsturgeon commented Jun 7, 2018 via email

@retrosight
Copy link
Author

@Relequestual Ready now for you to review and provide feedback before merging.

It looks quite wholesale on the surface yet most of the original content is still there. I tried to respect the intent of the original author(s) as much as I could.

It's going to be hard to read unless you are a diff hound.

Therefore I suggest you might want to navigate over to...

https://github.com/retrosight/json-schema-org.github.io/blob/master/learn/index.md

...and browse the complete collection as it will appear on the site.

@retrosight
Copy link
Author

I'll also note I made some changes to index.html at the root ___domain: https://github.com/retrosight/json-schema-org.github.io/blob/master/index.md -- removal of the telescope pointers there was requested by @handrews (and I agreed with him) and I believe these changes resolve #171.

@Relequestual
Copy link
Member

@retrosight Super thankyou for all the hard work you've clearly done on this. Sorry I wasn't able to review this when I intended. I have now started, but expect to finish today or at most end of next week.

@philsturgeon philsturgeon mentioned this pull request Jun 27, 2018
philsturgeon pushed a commit that referenced this pull request Jun 28, 2018
@philsturgeon
Copy link
Contributor

Merged in #221. Thank you!

@Relequestual
Copy link
Member

Sorry for being a terrible person. I was actually going to be looking at this today, but I guess I that was still too long to wait for the others.

Seriously thanks for your hard work on this. I'll read through the updates today anyway =]

@Relequestual
Copy link
Member

Great work. I have one small fix to apply and one followup issue to raise. But honestly this is super great! Thanks again!

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants