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

Read-Only JSON API Tutorial #8521

Closed
wants to merge 37 commits into from
Closed

Read-Only JSON API Tutorial #8521

wants to merge 37 commits into from

Conversation

izdwuut
Copy link

@izdwuut izdwuut commented Dec 23, 2020

This is a 🔦 documentation change.

Summary

I've added a tutorial on building Read-Only JSON APIs. It mostly covers creating a custom plugin with generators, converters and hooks. This is a detailed step-by-step guide.

I think it's great that you allow such contributions. I was about to put it behind a paywall on Medium, but if you add the tutorial to the docs, it could reach far more people.

Preview 👀

@izdwuut izdwuut changed the title Read-Only JSON API tutorial Read-Only JSON API Tutorial Dec 23, 2020
@ashmaroli ashmaroli requested review from a team, marti1125, MichaelCurrin and DirtyF and removed request for a team and marti1125 December 23, 2020 11:08
@DirtyF
Copy link
Member

DirtyF commented Dec 23, 2020

Note to self: Ideally our docs should have four separated parts:

  • tutorials (get a project done)
  • how-tos (get a task done)
  • reference (options, tags, filters,) with examples
  • explanations (high-level understanding of some concepts)

@izdwuut
Copy link
Author

izdwuut commented Dec 23, 2020

@DirtyF, I believe that my contribution would fall under the first category you've listed. Is there something I've got wrong?

@DirtyF
Copy link
Member

DirtyF commented Dec 24, 2020

@izdwuut Not at all, it's mostly a reminder for me to reevaluate how we should organize Jekyll documentation.

@MichaelCurrin
Copy link
Contributor

Thanks for the contribution.

Here is the deploy preview link for ease of reading.

https://deploy-preview-8521--jekyllrb.netlify.app/tutorials/read-only-json-api/

@MichaelCurrin
Copy link
Contributor

I wonder if the solution here is overly complicated. Especially at it relies on Ruby code as custom plugins, which is a barrier to setup and maintain. And can't be used on GH Pages due to restricted use of plugins.

For comparsion, I've made tutorial gist which uses builtin Jekyll functionality to make JSON files from a page and a post, without using any Ruby code. It might to everything covered in your case, but I think it much easier to understand and implement for the average Jekyll user.

https://gist.github.com/MichaelCurrin/f8d908596276bdbb2044f04c352cb7c7

@izdwuut
Copy link
Author

izdwuut commented Dec 25, 2020

I wonder if the solution here is overly complicated. Especially at it relies on Ruby code as custom plugins, which is a barrier to setup and maintain. And can't be used on GH Pages due to restricted use of plugins.

Yes, the fact that it can't be hosted as-is on GH Pages is a major downside of my approach. Theoretically you could generate your page yourself and put it on gh-pages branch (and keep your sources on another branch), but it adds yet another layer of complexity to already complex solution.

For comparsion, I've made tutorial gist which uses builtin Jekyll functionality to make JSON files from a page and a post, without using any Ruby code. It might to everything covered in your case, but I think it much easier to understand and implement for the average Jekyll user.

https://gist.github.com/MichaelCurrin/f8d908596276bdbb2044f04c352cb7c7

This is interesting. It didn't strike me at first that the problem can be tackled using layouts alone. The only thing I don't like about your approach is that you rely on many JSON files that, in reality, are either Markdown or HTML files - this is especially valid for posts. In VSCode - according to my knowledge - it would result in not being able to dynamically preview your post. Since it has a .json extension, it just wouldn't be recognized as a valid Markdown document - if I'm not wrong. In my solution, there is only one file that behaves like this - a default.html layout.

Would you mind if I linked to your Gist in my tutorial? To some, it could be a nice (and probably preferred) alternative.

Also, I'm going to introduce the rest of fixes in some time. I need to give myself some slack and configure my freshly acquired laptop.

@MichaelCurrin
Copy link
Contributor

MichaelCurrin commented Dec 25, 2020

Theoretically you could generate your page yourself and put it on gh-pages branch (and keep your sources on another branch), but it adds yet another layer of complexity to already complex solution.

What you can do is make a mention that your solution needs a non-standard environment to support the custom ruby code and then link to the GH Actions deploy tutorial on the Jekyll site. It's not under tutorials but you can figure out the path and use the link tag.

https://jekyllrb.com/docs/continuous-integration/github-actions/

I have some content here for that using other actions for your personal interest https://michaelcurrin.github.io/code-cookbook/recipes/ci-cd/github-actions/workflows/jekyll/

@MichaelCurrin
Copy link
Contributor

@izdwuut you can configure VS Code file associations.

You can do it temporarily for any given file (which will reset when you close an open the file). Click the language in the bottom right e.g. JSON and choose "Jekyll (HTML)" for liquid highlighting or Markdown (usually for .md files).
You can do it more permanently using Configure File Associations or similar in the command prompt menu.

You can also configure this in your project's .vscode/settings.json file so it applies to all JSON files but doesn't affect your other projects.

e.g.

{
    "files.associations": {
        "*.json": "jekyll"
    }
}

And yes please do. I'll keep the gist around so you are welcome to link to it. Thanks.

@izdwuut
Copy link
Author

izdwuut commented Dec 26, 2020

I've introduced all the changes. In one of the commits I've added a disclaimer where I inform about GitHub Actions and linked to your Gist.

Thanks for letting me know that assigning custom file associations is possible in VSCode!

@ashmaroli
Copy link
Member

@izdwuut I'd recommend adding one more disclaimer at the very end of the tutorial something along the lines of:

Disclaimer: The Ruby code in this tutorial is only for illustration purpose. The Jekyll team is not be liable to resolve
any bugs or issues arising from its use in production.

@izdwuut
Copy link
Author

izdwuut commented Dec 27, 2020

@ashmaroli I believe it belongs to the generic tutorials page. This could apply to any tutorial added in the future, so I think it's better to to put it there. What do you think?

@ashmaroli
Copy link
Member

I edited the tutorial so it relies on more templates

@izdwuut I still don't see _includes/post.rb being used anywhere..

@izdwuut
Copy link
Author

izdwuut commented Jan 6, 2021

@ashmaroli Right! I see what you mean now. This file should actually be post.html, as it's just a HTML template. Sorry for the confusion. At first I thought that the filename is right and I've just forgot to describe it.

@DirtyF I'm going to add it in a bit. I have changes to push and I'm not in front of my desktop PC right now.

@DirtyF
Copy link
Member

DirtyF commented Jan 6, 2021

@izdwuut No hurry, we'll publish when it's done 😄

@izdwuut
Copy link
Author

izdwuut commented Jan 7, 2021

I updated the description for ApiGenerator class to utilize a Ruby map instead of a regular loop. There also lied a bug, which I fixed.

I created the repo for the project. Any idea where to include it to have the most impact? Should it be a note or is it enough to link it in a place you've pointed to in your review, @DirtyF? Also, can I include some sane license like MIT?

@ashmaroli
Copy link
Member

@izdwuut You seem to have introduced typos to the document in the latest commit.
Regarding the repo, you can replace the directory structure in this tutorial with a link to the repo. Also, it can be whatever licence you wish. You are the author and owner of the repo.

@DirtyF
Copy link
Member

DirtyF commented Jan 7, 2021

@izdwuut I would mention the repository link from the beginning (setup for people to be able to follow through or for the impatient that would like to see the code directly.

Maybe you can invite people to clone the repository first and adapt the text copy?

And yes you can add an MIT LICENSE to your repository.

@izdwuut
Copy link
Author

izdwuut commented Jan 7, 2021

@DirtyF Yes, I think I can add a branch with barebone directories structure and ask users to clone it first.

@ashmaroli Sorry about that. I'll fix it in a moment.

Edit: I added the repository to the tutorial.

docs/_tutorials/read-only-json-api.md Outdated Show resolved Hide resolved
docs/_tutorials/read-only-json-api.md Outdated Show resolved Hide resolved
docs/_tutorials/read-only-json-api.md Outdated Show resolved Hide resolved
docs/_tutorials/read-only-json-api.md Outdated Show resolved Hide resolved
Let's dive in!

{: .note .info}
This solution needs a custom environment to support its Ruby code. This means that you can't host the site on GitHub
Copy link
Member

Choose a reason for hiding this comment

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

This entire paragraph needs rephrasing.

In particular, this line can be confusing. Hosting the source files on GitHub is different from deploying a static site via GitHub Pages.

Copy link
Author

Choose a reason for hiding this comment

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

I updated this paragraph. I think it's less confusing now.

docs/_tutorials/read-only-json-api.md Outdated Show resolved Hide resolved
docs/_tutorials/read-only-json-api.md Outdated Show resolved Hide resolved
docs/_tutorials/read-only-json-api.md Outdated Show resolved Hide resolved
"draft","layout","category","custom-property","slug","ext"]
```

Given that, I'd like you to enter the following template:
Copy link
Member

Choose a reason for hiding this comment

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

The use of the term enter in this line and the previous ones is incorrect.
Conventionally, one is instructed to enter a location or enter data into a document.
Entering a template can therefore be misinterpreted.

Additionally, it is not clear where to insert the subsequent Liquid code..

{% raw %}
```liquid
{
"title": {{ include.post.title | jsonify }},
Copy link
Member

Choose a reason for hiding this comment

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

what happens if one were to use "title": {{ page.title | jsonify }} instead?
`

Copy link
Author

Choose a reason for hiding this comment

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

A listing of posts under a category would render with null values.

@ashmaroli
Copy link
Member

@izdwuut I understand that English is not your first language. So, I'm converting this to a draft PR.
Feel free to take your time, finish the entire document, read through the entire tutorial by serving the docs site locally and then push the changes.

Ping us when you are ready.

@ashmaroli ashmaroli marked this pull request as draft January 7, 2021 15:39
DirtyF and others added 5 commits January 7, 2021 16:41
Co-authored-by: Ashwin Maroli <ashmaroli@users.noreply.github.com>
Co-authored-by: Ashwin Maroli <ashmaroli@users.noreply.github.com>
Co-authored-by: Ashwin Maroli <ashmaroli@users.noreply.github.com>
Co-authored-by: Ashwin Maroli <ashmaroli@users.noreply.github.com>
@izdwuut
Copy link
Author

izdwuut commented Jan 14, 2021

I'm terribly sorry for forcing you to review the PR in the previous state. I have to admit it - it was awful at places, so I'm thankful for your patience. I updated the tutorial according to your remarks. I read the tutorial a couple of times and made sure that people who follow it can actually yield the result. There still may be errors, but I did my best to get rid of most of them.

I hope it's okay if I ping just you, @ashmaroli?

izdwuut and others added 3 commits January 14, 2021 13:12
Co-authored-by: Ashwin Maroli <ashmaroli@users.noreply.github.com>
Co-authored-by: Ashwin Maroli <ashmaroli@users.noreply.github.com>
@ashmaroli
Copy link
Member

@izdwuut I'm sorry to force more edits. But recent commits seem to have removed chunks of text.
Please read through the section titled ### Dummy Posts...

@izdwuut
Copy link
Author

izdwuut commented Jan 14, 2021

@ashmaroli It's okay. It's your job, after all.

It's probably because I've had conflicts when merging and resolved the changes incorrectly. You're right - there are places where things are not right. I'll do my best to fix it in a reasonable time. It's not that we're in a hurry, but this discussion is exceptionally long. You probably have better things to do (and more important, too) than taking care of this tutorial for another week.

@izdwuut
Copy link
Author

izdwuut commented Jan 15, 2021

@ashmaroli I know why this problem arose. It's because there were some changes that you - the maintainers team - proposed, that I introduced after fixing the document locally. That's why there were missing things - merging through the GitHub interface went wrong because document versions didn't match.

Additionally, this time I've run the text through Grammarly. It found some mistakes, which I corrected. I'm sorry that I didn't do it earlier. It would save you some hassle. I also checked if every link works - and it does.

Comment on lines +24 to +26
* `/{url}` --- details of a post
* `/categories` --- an index of categories
* `/categories/{category}` --- an index of posts under a category
Copy link

Choose a reason for hiding this comment

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

Suggested change
* `/{url}` --- details of a post
* `/categories` --- an index of categories
* `/categories/{category}` --- an index of posts under a category
* `/{url}/` --- details of a post
* `/categories/` --- an index of categories
* `/categories/{category}/` --- an index of posts under a category

since AFAICT you cannot get rid of the slash:

$ curl -IL https://izdwuut.github.io/jekyll-json-api-tutorial/categories
HTTP/2 301 
server: GitHub.com
content-type: text/html
permissions-policy: interest-cohort=()
strict-transport-security: max-age=31556952
location: https://izdwuut.github.io/jekyll-json-api-tutorial/categories/
access-control-allow-origin: *
expires: Wed, 28 Jul 2021 15:46:05 GMT
cache-control: max-age=600
x-proxy-cache: MISS
x-github-request-id: A044:447C:5DE631:890EB7:61017965
accept-ranges: bytes
date: Wed, 28 Jul 2021 15:36:52 GMT
via: 1.1 varnish
age: 47
x-served-by: cache-fty21356-FTY
x-cache: HIT
x-cache-hits: 1
x-timer: S1627486612.323765,VS0,VE1
vary: Accept-Encoding
x-fastly-request-id: 18eb9201adfa28e4b3ee52de9c53090e57201a12
content-length: 162

HTTP/2 200 
server: GitHub.com
content-type: application/json; charset=utf-8
permissions-policy: interest-cohort=()
strict-transport-security: max-age=31556952
last-modified: Thu, 14 Jan 2021 11:45:10 GMT
access-control-allow-origin: *
etag: "60002ec6-15"
expires: Wed, 28 Jul 2021 15:45:47 GMT
cache-control: max-age=600
x-proxy-cache: MISS
x-github-request-id: 5514:447C:5DE3FF:890B33:61017953
accept-ranges: bytes
date: Wed, 28 Jul 2021 15:36:52 GMT
via: 1.1 varnish
age: 65
x-served-by: cache-fty21356-FTY
x-cache: HIT
x-cache-hits: 1
x-timer: S1627486612.347649,VS0,VE1
vary: Accept-Encoding
x-fastly-request-id: e56fa6c0ce7c97e157f7c7ab45c59c92e7fed259
content-length: 21

@izdwuut izdwuut closed this by deleting the head repository May 23, 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.

None yet

6 participants