Skip to content

ci: add experimental verification checks#444

Draft
AlexTzib wants to merge 2 commits into
Manuito83:developfrom
AlexTzib:ci/experimental-verification-checks
Draft

ci: add experimental verification checks#444
AlexTzib wants to merge 2 commits into
Manuito83:developfrom
AlexTzib:ci/experimental-verification-checks

Conversation

@AlexTzib
Copy link
Copy Markdown
Contributor

@AlexTzib AlexTzib commented May 4, 2026

DRAFT - do not merge yet

This PR is intentionally opened as a draft to test extra CI checkers before we decide whether they are stable enough to keep or require.

Summary

Adds three independent verification jobs:

  • Dart Format: checks formatting only for changed Dart files under lib/ and test/
  • Generated Code Drift: runs only when generator inputs changed, then runs dart run build_runner build --delete-conflicting-outputs and fails if git diff --exit-code finds generated changes
  • Dependency Review: runs actions/dependency-review-action@v4 on PRs and fails on newly introduced high/critical vulnerable dependencies

Why

The existing CI already checks analyze, tests, Cloud Functions, Android build, and iOS build. These extra jobs are meant to catch different classes of mistakes:

  • formatting drift in files touched by a PR
  • generated files that were not committed after model/API/env changes
  • risky dependency changes before merge

What the first draft run showed

The first version checked all Dart files and all generated outputs. That was too broad for the current repo baseline:

  • Dart Format tried to reformat 377 existing files
  • Generated Code Drift found existing swagger/env generated output drift

This PR was tuned so unrelated PRs are not blocked by existing baseline debt:

  • format now checks changed Dart files only
  • generated drift now runs only when generator input files change

Things to evaluate before merge

  • Whether checking only changed Dart files is enough for contributor hygiene
  • Whether the generated-input path list is broad enough
  • Whether Dependency Review should stay at fail-on-severity: high
  • Whether these checks should run on every push, PR only, or remain optional

Verification

  • git diff --check passed locally
  • This draft PR itself is the real test run for the new checkers

@AlexTzib
Copy link
Copy Markdown
Contributor Author

AlexTzib commented May 4, 2026

@Manuito83 small note from the experimental run: this is not directly related to this PR, but the new checks surfaced a few existing repo-maintenance topics that are probably worth discussing separately.

  1. Repo-wide Dart formatting drift: the first broad Dart Format run wanted to reformat 377 existing files. I tuned this PR to check changed Dart files only, so unrelated PRs are not blocked by existing formatting drift.

  2. Generated code drift: the first broad generated-code run changed swagger/env generated outputs. I tuned this PR so generated drift only runs when generator inputs change, but a separate generated-output refresh PR may be worth reviewing later.

  3. GitHub Actions Node 20 deprecation warning: GitHub warns that actions/checkout@v4 and actions/dependency-review-action@v4 currently run on Node 20. Not urgent for this PR, but worth tracking before GitHub forces Node 24 defaults.

@AlexTzib
Copy link
Copy Markdown
Contributor Author

AlexTzib commented May 4, 2026

What this CI experiment is meant to achieve:

  • Dependency Review gives us a PR-level safety net for dependency changes. If a PR introduces a high/critical vulnerable package, GitHub will flag it before merge.

  • Dart Format gives contributors quick feedback when changed Dart files are not formatted, without forcing a huge repo-wide formatting cleanup first.

  • Generated Code Drift catches cases where a PR changes generator inputs but forgets to commit the generated Dart outputs. That helps avoid “works locally after generation, broken from a clean checkout” situations.

The goal is not to make CI stricter for the sake of it. The goal is to catch common review-time misses automatically, while keeping the checks scoped enough that normal app PRs stay practical.

@Manuito83
Copy link
Copy Markdown
Owner

Thanks. A few thoughts:

  • Format check on changed files only: agree, let's keep it that way, I don't want to reformat the whole repo.

  • Generated drift: please exclude lib/models/api_v2/ from this check. I edited those files on purpose because Torn's swagger spec has been generated incorrectly in the past... we should sort this out properly someday.

  • Dependency review at high severity: sounds OK

  • PR-only runs: 100%

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.

2 participants