Change link checking setup #128
Conversation
WalkthroughReplaces the CI link-check step to run a Nix devShell helper ( Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor CI as CI / Developer
participant Nix as Nix devShell
participant Helper as check-external-links
participant Lychee as lychee
participant File as README.md
participant Net as External Sites
CI->>Nix: nix develop -c check-external-links
Nix->>Helper: launch helper (writeShellApplication)
Helper->>Lychee: lychee --cache --max-concurrency 2 --verbose README.md
Lychee->>File: parse README links
Lychee->>Net: request URLs (concurrent, cached)
Net-->>Lychee: responses
Lychee-->>Helper: report + exit status
Helper-->>Nix: return status
Nix-->>CI: command result
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches🧪 Generate unit tests
Tip 👮 Agentic pre-merge checks are now available in preview!Pro plan users can now enable pre-merge checks in their settings to enforce checklists before merging PRs.
Please see the documentation for more information. Example: reviews:
pre_merge_checks:
custom_checks:
- name: "Undocumented Breaking Changes"
mode: "warning"
instructions: |
Pass/fail criteria: All breaking changes to public APIs, CLI flags, environment variables, configuration keys, database schemas, or HTTP/GraphQL endpoints must be documented in the "Breaking Change" section of the PR description and in CHANGELOG.md. Exclude purely internal or private changes (e.g., code not exported from package entry points or explicitly marked as internal).Please share your feedback with us on this Discord post. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
flake.nix (1)
64-66: Optional: avoid duplicating Lychee in the devShellLychee is already provided via
runtimeInputsfor the helper; consider removing it from the top-levelpackageslist to slim the shell.- packages = with pkgs; [ - lychee + packages = with pkgs; [ (writeShellApplication { name = "check-external-links"; runtimeInputs = with pkgs; [ lychee ];
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
.github/workflows/publish.yml(1 hunks).gitignore(1 hunks)README.md(1 hunks)flake.nix(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Upload NixOS AMI (aarch64-linux, UbuntuLatest32Cores128GArm)
- GitHub Check: Upload NixOS AMI (x86_64-linux, ubuntu-latest)
🔇 Additional comments (3)
.gitignore (1)
1-2: LGTM: ignore Lychee cacheGood addition; prevents committing
.lycheecache.README.md (1)
88-88: LGTM: updated Terraform linkNew canonical URL looks good.
.github/workflows/publish.yml (1)
42-44: LGTM: use devShell helper for link checkingCleaner than invoking Lychee directly. Note: this depends on the flake helper being fixed per my flake.nix comment.
flake.nix
Outdated
| (writeShellApplication { | ||
| name = "check-external-links"; | ||
| runtimeInputs = with pkgs; [ lychee ]; | ||
| text = '' | ||
| lychee \ | ||
| --cache \ | ||
| --max-concurrency 2 \ # prevents 429 errors | ||
| --verbose \ | ||
| README.me | ||
| ''; | ||
| }) |
There was a problem hiding this comment.
Fix two bugs in link-check helper: wrong filename and broken line continuation
- Uses
README.meinstead ofREADME.md(will 404). - Backslash before an inline
#comment (--max-concurrency 2 \ # ...) breaks continuation and can inject a stray arg.
Apply:
- text = ''
- lychee \
- --cache \
- --max-concurrency 2 \ # prevents 429 errors
- --verbose \
- README.me
- '';
+ text = ''
+ set -euo pipefail
+ # Keep concurrency low to reduce 429s
+ lychee \
+ --cache \
+ --max-concurrency 2 \
+ --verbose \
+ README.md
+ '';📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| (writeShellApplication { | |
| name = "check-external-links"; | |
| runtimeInputs = with pkgs; [ lychee ]; | |
| text = '' | |
| lychee \ | |
| --cache \ | |
| --max-concurrency 2 \ # prevents 429 errors | |
| --verbose \ | |
| README.me | |
| ''; | |
| }) | |
| (writeShellApplication { | |
| name = "check-external-links"; | |
| runtimeInputs = with pkgs; [ lychee ]; | |
| text = '' | |
| set -euo pipefail | |
| # Keep concurrency low to reduce 429s | |
| lychee \ | |
| --cache \ | |
| --max-concurrency 2 \ | |
| --verbose \ | |
| README.md | |
| ''; | |
| }) |
🤖 Prompt for AI Agents
In flake.nix around lines 66 to 76, the link-check helper mistakenly references
README.me (should be README.md) and has a broken line continuation due to an
inline comment following a trailing backslash on the `--max-concurrency 2 \ #
...` line; change the filename to README.md and remove the inline `#` comment
from the same continued line (either move the explanatory comment to its own
line above or remove it) so the backslash properly escapes the newline and no
stray argument is injected.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
flake.nix (1)
102-107: Fix link checker: wrong filename; add safer shell options
README.mewill 404 (should beREADME.md). Also addset -euo pipefailto fail fast. Inline-comment/backslash issue noted previously is resolved—thanks.- text = '' - lychee \ + text = '' + set -euo pipefail + lychee \ --cache \ --max-concurrency 2 \ --verbose \ - README.me + README.md '';
🧹 Nitpick comments (3)
flake.nix (3)
22-33: Consider passingselfintoforSystemsclosuresForwarding
selfenables consumers to referenceself.*inside the per-system function without free-variable capture. Optional, but future-proof.Apply:
- forSystems = - systems: f: - inputs.nixpkgs.lib.genAttrs systems ( - system: - f { + forSystems = + systems: f: + inputs.nixpkgs.lib.genAttrs systems ( + system: + f { inherit system; pkgs = import inputs.nixpkgs { inherit system; }; - lib = inputs.nixpkgs.lib; + lib = inputs.nixpkgs.lib; + self = self; } );
42-76: Tighten assertion message for faster debuggingInclude the actual label in the failure to speed up diagnosis.
- message = "nixos image label is incorrect"; + message = "nixos image label is incorrect: ${toString config.system.nixos.label}";
85-116: Avoid duplicate lychee in the devShell
check-external-linksalready declareslycheeinruntimeInputs. Keepinglycheealso inpackagesis redundant unless you want directlycheeCLI access. If not needed, drop it.- [ - lychee + [ (writeShellApplication { name = "check-external-links"; runtimeInputs = with pkgs; [ lychee ];
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
flake.lockis excluded by!**/*.lock
📒 Files selected for processing (3)
.github/workflows/publish.yml(2 hunks)flake.nix(2 hunks)result(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- result
🚧 Files skipped from review as they are similar to previous changes (1)
- .github/workflows/publish.yml
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: Upload NixOS AMI (x86_64-linux, ubuntu-latest)
- GitHub Check: Upload NixOS AMI (aarch64-linux, UbuntuLatest32Cores128GArm)
- GitHub Check: Basic Nix checks
🔇 Additional comments (5)
flake.nix (5)
10-11: Outputs arg pattern LGTMUsing
{ self, ... } @ inputsis clear and flexible.
13-20: System sets look goodLinux/Darwin split is explicit and maintainable.
78-83: diskImages wiring LGTMDeriving
awsfrom the corresponding system’s amazonImage is correct.
120-125: apps pass-through LGTMRe-exporting
smoke-testfromnixos-amisper system is straightforward.
118-118: Confirm formatter availability across systems — manual verification requirednix not available in the verification environment (/bin/bash: nix: command not found). Run locally and paste the output:
#!/usr/bin/env bash set -euo pipefail for sys in x86_64-linux aarch64-linux x86_64-darwin aarch64-darwin; do echo "Checking formatter for $sys" nix eval --json ".#formatter.${sys}.pname" | jq -r . doneFile: flake.nix (line 118):
formatter = forAllSystems ({ pkgs, ... }: pkgs.nixfmt-rfc-style);
Summary by CodeRabbit
Documentation
Chores
Dev Experience