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

Uses Jekyll to generate the documentation #2536

Merged
merged 2 commits into from
Sep 16, 2024
Merged

Uses Jekyll to generate the documentation #2536

merged 2 commits into from
Sep 16, 2024

Conversation

st0012
Copy link
Member

@st0012 st0012 commented Sep 10, 2024

Motivation

Compared to RDoc, Jekyll is a much better platform for non-API-focused documentation. Some useful features for us are:

  • Rich themes to choose from
  • Easily-customizable templates
  • Easy configuration with _config.yml
  • Feature-rich markdown syntax support (e.g. supporting html tags too)

Implementation

  • Uses Jekyll to generate the documentation, which's source and configs live in the /jekyll folder.
  • The documentation will still be built to the /docs folder and be ignored by git.
  • Currently, the documentation is one big page (similar to rust-analyzer's).
  • All the current feature demos and introduction are copied from the Code Navigation post on the Rails At Scale blog. We will gradually fill in the rest of the documentation.
  • To avoid duplication, most of README's content has been moved to the documentation too.
  • Because Jekyll server isn't able to link to assets outside of the source directory, we need to copy the Ruby LSP icon to /jekyll folder.

Demo

Screen.Recording.2024-09-10.at.21.44.44.mov

Automated Tests

Manual Tests

@st0012 st0012 added the documentation Improvements or additions to documentation label Sep 10, 2024
@st0012 st0012 requested a review from a team as a code owner September 10, 2024 20:44
@st0012 st0012 requested review from andyw8 and vinistock September 10, 2024 20:44
@st0012 st0012 force-pushed the jekyll branch 2 times, most recently from b59a674 to a9f32ac Compare September 10, 2024 20:47
@st0012 st0012 added the server This pull request should be included in the server gem's release notes label Sep 10, 2024
@andyw8
Copy link
Contributor

andyw8 commented Sep 11, 2024

Saw these mentioned yesterday, could be useful if we want to use Tailwind for styling.

https://github.com/skatkov/jekyll-tailwind-cli-template
https://github.com/crbelaus/jekyll-tailwind

.github/workflows/publish_docs.yml Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
jekyll/Gemfile Outdated Show resolved Hide resolved
jekyll/Gemfile Outdated Show resolved Hide resolved
jekyll/Gemfile Show resolved Hide resolved
CONTRIBUTING.md Outdated Show resolved Hide resolved
jekyll/_config.yml Outdated Show resolved Hide resolved
@andyw8
Copy link
Contributor

andyw8 commented Sep 11, 2024

Is there somewhere else we could host the videos to avoid bloating the repo? They don't take up much space currently, but each time we update them the repo will grow.

@andyw8
Copy link
Contributor

andyw8 commented Sep 11, 2024

Once shipped, we should link to this from https://shopify.github.io/ to help with SEO.

It's already there.

@st0012
Copy link
Member Author

st0012 commented Sep 12, 2024

Is there somewhere else we could host the videos to avoid bloating the repo? They don't take up much space currently, but each time we update them the repo will grow.

Vini and I talked about this before, specifically with Git LFS as the potential solution. But IMO we should focus on improving the content until RailsWorld and then worry about this later.

Copy link
Member

@vinistock vinistock left a comment

Choose a reason for hiding this comment

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

Looks good to me. I left a couple of comments regarding the content, but I would rather have we move forward with the platform changes and then iterate on the content than holding this PR back

jekyll/_config.yml Outdated Show resolved Hide resolved
jekyll/_config.yml Show resolved Hide resolved
@st0012 st0012 force-pushed the jekyll branch 6 times, most recently from aca1187 to 0c54044 Compare September 13, 2024 19:41
@st0012
Copy link
Member Author

st0012 commented Sep 13, 2024

@vinistock because sass-embedded, a dependency of jekyll, fails to build with Ruby 3.1 and takes significantly longer to build with 3.2 (due to its build script relying on psych 5.1+, which only uses the version bundled with Ruby). Since Jekyll is only needed for documentation, I don't want to complicate the CI setup for it.
Given that ruby-lsp doesn't have dependabot working atm anyway (due to rbi requires Ruby 3.1 constantly failing it), can we maintain a separate Gemfile for Jekyll for now?

@vinistock
Copy link
Member

@st0012 would this work?

  • Put jekyll related gems in an optional Gemfile group, like group: :documentation
  • Use The setup-ruby's action bundle config through environment variables to only include the group when generating the docs. It should just be a matter of using "BUNDLE_WITH": "documentation" in the config

- Uses Jekyll to generate the documentation, which's source and configs
  live in the `/jekyll` folder.
- The documentation will still be built to the `/docs` folder and be
  ignored by git.
- Currently, the documentation is one big page (similar to rust-analyzer's).
- All the current feature demos and introduction are copied from the
  Code Navigation post on the Rails At Scale blog. We will gradually
  fill in the rest of the documentation.
- To avoid duplication, most of README's content has been moved to the
  documentation too.
- Because Jekyll server isn't able to link to assets outside of the source
  directory, we need to copy the Ruby LSP icon to `/jekyll` folder.
Jekyll introduces a lot of dependencies that we don't need for developing
Ruby LSP. Having a separate Gemfile for it should make dependencies easier
to understand and manage.
@st0012 st0012 enabled auto-merge (squash) September 16, 2024 14:39
@st0012 st0012 merged commit c8dcd55 into main Sep 16, 2024
37 checks passed
@st0012 st0012 deleted the jekyll branch September 16, 2024 14:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation server This pull request should be included in the server gem's release notes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants