V1.2.0

[vincerubinetti]

• add discussion templates. be more aggressive requiring fields.
• update to latest version of all github actions (checking for breaking changes)
• add debug step to each workflow to allow ssh access in
• fix site.location vs page.url bug (see issue)
• add html-proofer plugin to check for broken images and links, disabled by default because it adds a lot of time to hot-reload, but set explicitly as false in config so users can easily flip it on
• update bundler version and re-bundle gems
• add "remove" option for cite-process that allows a user to remove a source from a metasource. e.g., an ORCID returns a source that the user doesn't want on their site, so in their sources.yaml they put the id and remove: true.
• by default, ignore sources that manubot can't cite if they're from metasources (see changelog)
• add markdownify to more component params
• fix bug in citation (and other) components where when lookup is blank, first entry in list is always chosen
• make use of newly widely available CSS container queries
• remove compilation-time wrapping of tables because it breaks nested tables, and is not (practically) possible to support them in the same way. replace with run-time js plugin for wrapping.
• upgrade font awesome and improve loading mechanism
• fix header title/subtitle CSS bug reported by @Maruuka
• add "image" param to post excerpt component, so blog posts can have thumbnails
• fix bug that prevents manual passing of tags to tag component
• add heading 5/6 styles/support
• tweak dark mode plugin
• various style tweaks

New template version checklist:

• I have updated CITATION and CHANGELOG as appropriate.
• I have updated lab-website-template-docs as appropriate.
• I have checked the testbed as appropriate.
• I have tested changed workflows on a freshly generated and forked repo.

[github-actions]

PR Preview Action v1.4.7
Preview removed because the pull request was closed.
2024-03-08 21:30 UTC

[vincerubinetti]

This is ready for review. Regarding the failing pull request preview workflow, I'm fairly certain that this is due to GitHub wanting to run the version of the workflow that is on main, not this branch. If you look at the failing logs, it says Run ruby/setup-ruby@v1 with: ruby-version: 3.0, which is what is currently on main, instead of 3.1 which I've set in this PR. Weirdly, if I clear the gh-actions Ruby cache and re-run the failed job, it seems to pass, but then the next run fails.

Regardless, I've tested this on my own repo, as if the v1.2.0 changes were already on main, and it seems to work fine:
https://github.com/vincerubinetti/lwt-test/pull/1
The other workflows also seem to work fine with the version upgrades.

So, I think it is safe to ignore the failure, as it should work properly once merged into main.

[falquaddoomi]

This is ready for review. Regarding the failing pull request preview workflow, I'm fairly certain that this is due to GitHub wanting to run the version of the workflow that is on main, not this branch. If you look at the failing logs, it says Run ruby/setup-ruby@v1 with: ruby-version: 3.0, which is what is currently on main, instead of 3.1 which I've set in this PR. Weirdly, if I clear the gh-actions Ruby cache and re-run the failed job, it seems to pass, but then the next run fails.

Regardless, I've tested this on my own repo, as if the v1.2.0 changes were already on main, and it seems to work fine: vincerubinetti/lwt-test#1 The other workflows also seem to work fine with the version upgrades.

So, I think it is safe to ignore the failure, as it should work properly once merged into main.

About it running the version in main and not your PR branch: what you said is correct; because you're using pull_request_target as the event, by design it runs in the base branch (i.e., not the PR branch). From the docs on pull_request_target:

This event runs in the context of the base of the pull request, rather than in the context of the merge commit, as the pull_request event does. This prevents execution of unsafe code from the head of the pull request that could alter your repository or steal any secrets you use in your workflow. This event allows your workflow to do things like label or comment on pull requests from forks. Avoid using this event if you need to build or run code from the pull request.

Anyway, I'll ignore the failure like you said. I'll review this ASAP, too.

[vincerubinetti]

Yes, I temporarily tried adding the regular pull_request target here:
b5cacf2

But that also seemed to not work for some reason, unless I was looking at the wrong workflow run.

Based on testing updating the greenelab.com website, adding a pull_request trigger to on-pull-request.yaml (in the same manner as pull_request_target) works. Any user wanting to have a live deploy preview for a PR when updating their site to v1.2.0 can temporarily add this, then remove it again before merge.

Regardless, we need pull_request_target because in previous testing it proved to be more reliable when forking the template, and we cannot have both triggers because it will create confusing duplicate checks on PRs (and one of them usually fails while the other one passes, depending on whether the change is being made from a branch or a fork).

[d33bs]

Great work! I left a few comments, many of them optional/minor. Let me know if you have any questions.

Along the way I wondered if a CSS or SCSS linter could assist over time (if they're not already present). In addition, perhaps a Liquid linter could help in a similar way if it exists. Overall I have gaps in my understanding for those, so it could be that this is impractical or not relevant. I bring it up as a thought to strengthen the work and am not concerned (I think you did a good job on these things from what I can tell).

[vincerubinetti]

Great work! I left a few comments, many of them optional/minor. Let me know if you have any questions.

Along the way I wondered if a CSS or SCSS linter could assist over time (if they're not already present). In addition, perhaps a Liquid linter could help in a similar way if it exists. Overall I have gaps in my understanding for those, so it could be that this is impractical or not relevant. I bring it up as a thought to strengthen the work and am not concerned (I think you did a good job on these things from what I can tell).

As far as I know, there's no actively maintained linter for liquid. I did investigate using a liquid formatter at least, which should be somewhere in the issue/pr history, but I couldn't get it to work reliably with the mix of markdown, html, and liquid.

Regarding CSS, I do have IDE extensions that will tell me for example if I've written an invalid CSS property or value for it, but of course that doesn't cover other people who might maintain it in the future. Perhaps I could add a .vscode folder with recommended extensions and settings at some point. It would also be possible to set up PR tests for linting and such, however the template really hasn't opened that can of worms yet for anything else. It's assumed most users won't change the template internals (css/js/etc), and if they do, it's on them. Bluntly, Jekyll/Liquid/Ruby imo are just not really the right or modern tools for the job. Simply using JSX and TS and components with a framework like Astro would alleviate so many problems, e.g. automatic documentation for what components take what props.

[vincerubinetti]

Also I forgot to mention, I have a gitbook "pr" ready to merge for these changes. I believe you should be able to see this preview link:

https://greene-lab.gitbook.io/lab-website-template-docs/~/changes/i7RDBjuWCIkfNmYxvoTx/introduction/overview

[d33bs]

Thanks for addressing all the comments, and again, great work - LGTM! I left a minor/optional follow up on the composite actions item just in case.

[falquaddoomi]

Nice work, and it seems like there's already been plenty of discussion about possible issues that, as far as I can tell, has been resolved. I took a quick look through the code and didn't see anything that stood out.