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

Upgrade to Jekyll 4.3 #273

Merged
merged 3 commits into from
Jul 28, 2023
Merged

Upgrade to Jekyll 4.3 #273

merged 3 commits into from
Jul 28, 2023

Conversation

BryanQuigley
Copy link
Collaborator

Also updated bundler and ruby versions in Docker.

Had to go through several iterations to get GitHub-metadata to work, as part of that:

With the switch to GitHub actions/Netlify I don't see any blockers to the upgrade.

Also updated bundler and ruby versions in Docker.

Had to go through several iterations to get GitHub-metadata to work, as part of that:
 * Dropped Open in GitHub button from categories page as we don't generally want edits there.
 * GitHub edit link didn't work initially so reverted pending fix for jekyll/github-metadata#249.
@timwis
Copy link
Owner

timwis commented Apr 19, 2023

Looking good! And I agree, it's great that we can let go of the github-pages plugin. The only thing is that that plugin also bundles in a variety of other plugins, so we'll want to review those and consider whether anyone may be depending on them (if so, this would be a breaking change for them as they'd have to explicitly declare that dependency or their build will break). The easy way around that is to simply bundle those plugins in JKAN as well (until the next breaking-change version).

Here's the list of plugins. I think it's just the first set at the top we'd need to worry about, as I presume the list after that still need to be explicitly declared when using the github-pages gem.

The only other thing is that pesky "Edit on GitHub" link, which ideally wouldn't be statically bound to the main branch. It's driving me mad trying to figure out why that isn't working properly... I notice that, even without this PR, those links still point to gh-pages in my local development environment for some reason, even though we've changed the default branch. I reckon it's fine to merge without that fix since it's broken to begin with, but we should create an issue for it if we don't fix it before merging this.

@timwis
Copy link
Owner

timwis commented Apr 19, 2023

I just ran into what I believe to be an example of the above dependency issue (though I could be wrong - I was testing with Jekyll 3.9). I assume this is pulled in by one of those plugins 🤔
Screenshot 2023-04-19 at 07 49 38

@timwis
Copy link
Owner

timwis commented Apr 19, 2023

Interesting: I've been investigating the branch issue a bit further, and it appears it may be an issue with the GitHub API, itself.

GET https://api.github.com/repos/timwis/jkan/pages (api docs)

returns:

{
  "url": "https://api.github.com/repos/timwis/jkan/pages",
  "status": "built",
  "cname": null,
  "custom_404": false,
  "html_url": "http://timwis.com/jkan/",
  "build_type": "workflow",
  "source": {
    "branch": "gh-pages",
    "path": "/"
  },
  "public": true,
  "protected_domain_state": null,
  "pending_domain_unverified_at": null,
  "https_certificate": {
    "state": "approved",
    "description": "The certificate has been approved.",
    "domains": [
      null,
      "www.timwis.com"
    ],
    "expires_at": "2023-07-05"
  },
  "https_enforced": false
}

Running it against azavea/opendataphilly-jkan says the source.branch is v2.

This is the value that the github-pages gem is using to construct the edit link.

These were included as part of GH pages in 3.9.
@BryanQuigley
Copy link
Collaborator Author

The GitHub edit link changing has been annoying me to no end, thank you for chasing it down and figuring out where it actually was coming from!

I was hoping it would be simple to just include them, but just checking one github/jekyll-commonmark-ghpages#22 determined it won't be an obvious 1:1 matching..

  • jekyll-coffeescript | doesn't seem maintained/updated for 4.x - https://github.com/jekyll/jekyll-coffeescript
  • jekyll-commonmark-ghpages | I think we are fine just using kramdown
  • jekyll-gist | not active maintenance, should be easy to check if used by a site
  • jekyll-github-metadata | upgraded to similar
  • jekyll-paginate | seems used (but didn't confirm, so upgraded)
  • jekyll-relative-links | added
  • jekyll-optional-front-matter | compatible but I don't think this is useful for us.
  • jekyll-readme-index | compatible but I don't think this is useful for us.
  • jekyll-default-layout | compatible but I don't think this is useful for us.
  • jekyll-titles-from-headings | added because seems fine

As for your kramdown-parser-gfm error, that is included in the lock file by default, if you were trying to run it on Jekyll 3.9 I don't think it will work.

Copy link
Collaborator

@lydiascarf lydiascarf left a comment

Choose a reason for hiding this comment

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

pulled + tested that everything still runs alright and the Open in GitHub links now work 🎉

I'd like to test pulling this into ODP but I'm not sure of the best way to do that as of right now

@BryanQuigley BryanQuigley merged commit 3dcdfc8 into main Jul 28, 2023
@BryanQuigley BryanQuigley deleted the upgrade_jekyll branch September 5, 2023 22:33
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.

3 participants