Skip to content

Add PR build test and markdown lint workflows#65

Closed
JohnRDOrazio wants to merge 4 commits intoopensourcecatholic:masterfrom
JohnRDOrazio:add-pr-ci-checks
Closed

Add PR build test and markdown lint workflows#65
JohnRDOrazio wants to merge 4 commits intoopensourcecatholic:masterfrom
JohnRDOrazio:add-pr-ci-checks

Conversation

@JohnRDOrazio
Copy link
Copy Markdown
Contributor

@JohnRDOrazio JohnRDOrazio commented Apr 22, 2026

Closes #66.

Summary

Currently nothing runs on a PR before merge — breakage only surfaces post-merge via the Build & Deploy to GitHub Pages action on master. This PR adds two pull-request workflows so PRs get validated in flight:

  • pr-build-test.yml — runs bundle exec jekyll build on PRs that touch build-influencing files.
  • pr-markdown-lint.yml — runs markdownlint-cli2 on Markdown files changed in the PR.

Stacks on top of #63 (it assumes the Ruby 3.4.9 container and regenerated Gemfile.lock from that PR). Until #63 merges, the diff view on this PR will include those files too; once #63 lands I'll rebase and the diff collapses to just the three new files.

Design choices

Build test is path-filtered, not on every PR

Watches the files that can actually break a Jekyll build: .ruby-version, Gemfile, Gemfile.lock, _config.yml, _data/**, _includes/**, _layouts/**, _plugins/**, and the workflow itself. Editing a blog post doesn't trigger it — that's what the lint workflow is for.

Reuses the same ruby:3.4.9-bookworm container and the same vendor/bundle cache key as the deploy workflow (${{ runner.os }}-gems-${{ hashFiles('**/Gemfile.lock', '.ruby-version') }}). GitHub Actions cache falls through from the PR branch to the base branch, so PR builds warm-start from the cache that master's deploy already populated.

Markdown lint is scoped to changed files

Running markdownlint-cli2 against the whole tree would surface violations in years of existing posts on day one. Instead the workflow computes the changed .md files with git diff --name-only --diff-filter=d "$BASE_SHA"...HEAD -- '*.md' (where BASE_SHA is github.event.pull_request.base.sha) and passes only those paths to the linter. Rules apply going forward; old content only gets checked if/when someone edits it.

Newline-delimited xargs -d '\n' -r handles filenames robustly and skips the run entirely if the list is empty (belt-and-suspenders with the explicit [ -z ] check).

actions/checkout is invoked with fetch-depth: 0 because the default shallow clone doesn't include the base commit needed for the diff.

Lint rules

.markdownlint-cli2.yaml starts from default: true and adjusts two rules:

  • MD013 (line-length) — off. Prose content with long sentences shouldn't fail CI over wrapping.
  • MD033 (no-inline-html) — off. Posts on this site embed raw HTML (images, paragraphs, figures) frequently.

Notable rules kept enabled from defaults:

  • MD034 no-bare-urls — explicit feedback from the repo owner on a past PR: bare links shouldn't end up in posts.
  • MD040 fenced-code-language — code blocks should declare a language.

Everything else stays at the default — easy to tighten or loosen in follow-up PRs.

Action versions — pinned to full SHAs

Consistent with #63. New pin added:

  • actions/setup-node@48b55a0… (v6.4.0)

markdownlint-cli2@0.22.1 is pinned in the workflow via npm install -g. Bumps are manual for now; if autoupdates matter, future work is to introduce package.json + an npm entry in Dependabot.

Test plan

  • Merge this PR and confirm both workflows show up as required/optional checks on subsequent PRs.
  • Open a trivial test PR that edits a Markdown file — confirm PR markdown lint runs and PR build test does not.
  • Open a trivial test PR that touches _config.yml — confirm PR build test runs.

🤖 Generated with Claude Code

JohnRDOrazio and others added 3 commits April 22, 2026 21:09
- Pin actions/checkout, actions/cache, and peaceiris/actions-gh-pages
  to full commit SHAs (with version comments) to harden the supply
  chain and make updates reviewable
- Bump actions to current major versions (checkout v6, cache v5,
  actions-gh-pages v4), resolving the "Set up job" failure caused by
  actions/cache@v1 running on a retired Node runtime
- Bump the Ruby container from 3.2.2 to 3.4.9 (latest 3.x)
- Add .github/dependabot.yml with weekly updates for github-actions
  and bundler ecosystems so pinned SHAs and gem versions stay current

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The workflow container was bumped from 3.2.2 to 3.4.9, but the rest
of the repo still pointed at 3.2.2. Three follow-ups:

- Update .ruby-version from 3.2.2 to 3.4.9 so local tooling (rbenv,
  asdf, chruby) matches the CI container.
- Regenerate Gemfile.lock under Ruby 3.4.9. The old lockfile was
  BUNDLED WITH 2.2.22, which is incompatible with Ruby 3.4's
  DidYouMean API; the new one is BUNDLED WITH 2.6.9. Gem versions
  were resolved fresh.
- Expand the actions/cache key to include .ruby-version so that
  future Ruby bumps invalidate the gem cache, avoiding silent ABI
  mismatches between cached native extensions and a new Ruby
  runtime.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Adds two pull_request workflows so PRs get validated before merge:

- pr-build-test.yml runs bundle exec jekyll build whenever a PR
  touches build-influencing files (.ruby-version, Gemfile, Gemfile.lock,
  _config.yml, _data/**, _includes/**, _layouts/**, _plugins/**, or the
  workflow itself). Reuses the ruby:3.4.9-bookworm container from the
  deploy workflow and shares the same vendor/bundle cache key so PR
  builds warm-start from master's cache.

- pr-markdown-lint.yml runs markdownlint-cli2 on PRs touching *.md
  files, but only against files changed in the PR (diffed via
  github.event.pull_request.base.sha...HEAD). Lets rules land without
  requiring a mass-fix pass over existing posts.

- .markdownlint-cli2.yaml starts from `default: true` — enables
  MD034 no-bare-urls (explicit repo-owner feedback) and MD040
  fenced-code-language among other defaults. Disables MD013
  (line-length, disruptive for prose) and MD033 (no-inline-html,
  Jekyll posts frequently embed raw HTML).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Node 24 is the current active LTS line.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Copy link
Copy Markdown
Collaborator

@mkasberg mkasberg left a comment

Choose a reason for hiding this comment

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

Looks like this PR is stacked with your other one. The two workflow files in the last two commits look good to me!

node-version: '24'

- name: Install markdownlint-cli2
run: npm install -g markdownlint-cli2@0.22.1
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

This is fine... Nitpicking, but if it were up to me I'd simplify:

  • Since this project already uses ruby, use a gem instead like markdownlint.
  • Instead of running this as a separate workflow, just add a step to the other workflow (before or after the jekyll build) to lint markdown.
  • Get rid of the path guards on both workflows. Presumably, most PRs are changing either markdown or Jekyll files. It's simpler (and less error-prone in the future) to just run a single test workflow on every PR.

Don't feel too strongly though, what's here will work 🤷‍♂️

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.

No pre-merge validation on PRs — build breakage and Markdown issues only surface post-merge

2 participants