search

ysk24ok / jekyll-linkpreview

Jekyll plugin to generate link preview
https://rubygems.org/gems/jekyll-linkpreview
MIT License
30 stars 8 forks source link

Make plugin aware of site source directory #32

Closed Clpsplug closed 1 year ago

Clpsplug commented 1 year ago

This change adds @source_dir, which is the 'source' value of the _config.yml file. This closes #31.

With this change, I confirmed with a blank Jekyll site that:

@source_dir is implemented as an instance var because it can change as opposed to @@template_dir, which I thought indicated a constant, but it can also be implemented as a class var.

I'm unsure how I handled the RSpec tests; I'm unfamiliar with development using Ruby, but I tried copying what the existing tests did and confirmed that all the tests passed.
The added case tests for rendering custom OGP template with valid OG information when the source config is modified. If this one case passes, so should all others.

If we should also test all cases for modified 'source' config or if there's something else I messed up in the tests, I'd be happy to fix them!

Manual tests

  1. Create a blank Jekyll site
  2. Install jekyll-linkpreview locally and add it to the site's Gemfile and _config.yml
  3. Modify the site's source files location as the following values and repeat 4. and 5.:
    • source: "_content"
    • source: "."
    • Do not put the source key, i.e., using Jekyll's default which is Dir.pwd
  4. Put the custom template, i.e., _includes/linkpreview{,_nog}.html, under the site's source.
  5. jekyll serve and confirm the following:
    • Both custom templates are rendered.
    • Changes to both templates are picked up by Jekyll, and reloading the browser shows them.

Test cases

Clpsplug commented 1 year ago

Thank you for your review; I'll work on modifications!

Clpsplug commented 1 year ago

Hey @ysk24ok, I made the changes! I hope it appropriately reflects your intentions.

ysk24ok commented 1 year ago

Sorry for being sporadic, but I added GitHub Actions by #33, so could you merge the master branch into this PR so we can make sure tests succeeds in every Ruby x Jekyll version?

Clpsplug commented 1 year ago

Sorry for being sporadic, but I added GitHub Actions by #33, so could you merge the master branch into this PR so we can make sure tests succeeds in every Ruby x Jekyll version?

I think I merged it correctly, but it seems GitHub Actions from forks refuse to run for a good reason.
Could you check here https://docs.github.com/en/actions/managing-workflow-runs/approving-workflow-runs-from-public-forks and take the required steps?

ysk24ok commented 1 year ago

Could you check here https://docs.github.com/en/actions/managing-workflow-runs/approving-workflow-runs-from-public-forks and take the required steps?

I can't see such "Approve and run" button, maybe because the workflows didn't exist when this PR was created. But then fine, if running tests locally succeeds, we can merge this PR.