chore: exclude generated files from git to eliminate merge conflicts#80
Merged
Conversation
Agent-Logs-Url: https://github.com/nitrocode/token-deathclock/sessions/e59e531c-78e8-4c77-806a-43871027c9b9 Co-authored-by: nitrocode <7775707+nitrocode@users.noreply.github.com>
Copilot created this pull request from a session on behalf of
nitrocode
April 26, 2026 15:38
View session
Contributor
📝 WalkthroughWalkthroughRemove several generated frontend artifacts from source control, update CI/workflows and npm scripts to build those artifacts during CI and preview runs, add build artifacts to .gitignore, and update docs to require local build before E2E/testing. Deleted large generated JS/CSS runtime files. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
Contributor
👁️ PR Preview
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
.github/workflows/e2e-tests.yml (1)
34-55:⚠️ Potential issue | 🟠 MajorBuild step is good, but the E2E trigger filter now misses real site changes.
Line 36 gates the whole job, and current filters don’t include
CHANGELOG.md,milestones.yaml, orproject-stats.yaml. With generated files no longer committed, those inputs can change runtime content without running E2E.🛠️ Suggested filter update
- name: Detect relevant file changes uses: dorny/paths-filter@fbd0ab8f3e69293af611ebaee6363fc25e6d187d # v4.0.1 id: filter with: filters: | code: - '**/*.js' - '**/*.ts' - '**/*.css' + - 'CHANGELOG.md' + - 'milestones.yaml' + - 'project-stats.yaml' - 'tests/**' - 'package*.json'🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/e2e-tests.yml around lines 34 - 55, The test-e2e job is gated by "if: needs.changes.outputs.code == 'true'" which misses non-code files (CHANGELOG.md, milestones.yaml, project-stats.yaml); update the workflow so test-e2e runs when those files change as well by either (A) extending the condition on the test-e2e job (replace "if: needs.changes.outputs.code == 'true'" with a compound check ORing outputs for the changelog/milestones/project-stats e.g. needs.changes.outputs.code == 'true' || needs.changes.outputs.changelog == 'true' || needs.changes.outputs.milestones == 'true' || needs.changes.outputs.project_stats == 'true'), or (B) modify the changes-detection step that produces needs.changes outputs to emit additional outputs for CHANGELOG.md, milestones.yaml, and project-stats.yaml so the existing if can reference them; locate "test-e2e" and the "if: needs.changes.outputs.code" line to implement the change.
🧹 Nitpick comments (1)
AGENTS.md (1)
322-336: One instruction still conflicts with the new E2E requirement.Line 134 now correctly requires
npm run build && npm run test:e2e, but Line 330 still says onlynpm run test:e2e. Please align the “What NOT to Do” bullet to avoid mixed guidance.🛠️ Suggested wording fix
-- Do **not** finish a session without running `npm run test:ci` and `npm run test:e2e` to confirm both suites pass. +- Do **not** finish a session without running `npm run test:ci` and `npm run build && npm run test:e2e` to confirm both suites pass.As per coding guidelines: "Document and maintain agent specifications in AGENTS.md file".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@AGENTS.md` around lines 322 - 336, The "What NOT to Do" bullet that currently instructs running only `npm run test:e2e` conflicts with the updated requirement to run a build first; edit the AGENTS.md bullet that reads "Do not finish a session without running `npm run test:ci` and `npm run test:e2e`" to require the build step by changing it to require `npm run build && npm run test:e2e` (or mirror the existing line that requires `npm run build && npm run test:e2e`) so both bullets consistently mandate the build before e2e tests.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In @.github/workflows/e2e-tests.yml:
- Around line 34-55: The test-e2e job is gated by "if:
needs.changes.outputs.code == 'true'" which misses non-code files (CHANGELOG.md,
milestones.yaml, project-stats.yaml); update the workflow so test-e2e runs when
those files change as well by either (A) extending the condition on the test-e2e
job (replace "if: needs.changes.outputs.code == 'true'" with a compound check
ORing outputs for the changelog/milestones/project-stats e.g.
needs.changes.outputs.code == 'true' || needs.changes.outputs.changelog ==
'true' || needs.changes.outputs.milestones == 'true' ||
needs.changes.outputs.project_stats == 'true'), or (B) modify the
changes-detection step that produces needs.changes outputs to emit additional
outputs for CHANGELOG.md, milestones.yaml, and project-stats.yaml so the
existing if can reference them; locate "test-e2e" and the "if:
needs.changes.outputs.code" line to implement the change.
---
Nitpick comments:
In `@AGENTS.md`:
- Around line 322-336: The "What NOT to Do" bullet that currently instructs
running only `npm run test:e2e` conflicts with the updated requirement to run a
build first; edit the AGENTS.md bullet that reads "Do not finish a session
without running `npm run test:ci` and `npm run test:e2e`" to require the build
step by changing it to require `npm run build && npm run test:e2e` (or mirror
the existing line that requires `npm run build && npm run test:e2e`) so both
bullets consistently mandate the build before e2e tests.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 97ff9992-3881-4b92-8049-29d7d40fe524
📒 Files selected for processing (9)
.github/workflows/e2e-tests.yml.github/workflows/preview.yml.gitignoreAGENTS.mdchangelog-data.jsmilestones-data.jsproject-stats-data.jsscript.jsstyles.css
💤 Files with no reviewable changes (5)
- project-stats-data.js
- styles.css
- milestones-data.js
- changelog-data.js
- script.js
Agent-Logs-Url: https://github.com/nitrocode/token-deathclock/sessions/3685802f-7bb8-4485-b875-03483b4367e5 Co-authored-by: nitrocode <7775707+nitrocode@users.noreply.github.com>
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #80 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 1 1
Lines 233 233
Branches 107 107
=========================================
Hits 233 233
Flags with carried forward coverage won't be shown. Click here to find out more. Continue to review full report in Codecov by Sentry.
🚀 New features to boost your workflow:
|
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (2)
package.json (1)
8-10: Extract the shared pre-test build chain, fool.Line 8 and Line 10 duplicate the same command. Put that chain in one script so local and CI hooks don’t drift later.
♻️ Suggested cleanup
"scripts": { "typecheck": "tsc --noEmit", - "pretest": "npm run build:milestones && npm run build:js", + "build:test-artifacts": "npm run build:milestones && npm run build:js", + "pretest": "npm run build:test-artifacts", "test": "jest --coverage", - "pretest:ci": "npm run build:milestones && npm run build:js", + "pretest:ci": "npm run build:test-artifacts", "test:ci": "jest --ci --coverage",🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@package.json` around lines 8 - 10, The pretest build chain is duplicated in the "pretest" and "pretest:ci" scripts; add a single shared script (e.g., "pretest:build") with the command "npm run build:milestones && npm run build:js", then update both "pretest" and "pretest:ci" to call "npm run pretest:build" so the build chain is centralized and cannot drift..github/workflows/unit-tests.yml (1)
51-58: I pity the fool who makes CI do the same work twice, sucka!Line 52 runs
npm run build:milestones && npm run build:js, and then line 58 callsnpm run test:ciwhile yourpackage.jsonalready definespretest:ciwith that exact same build chain. When you runnpm run test:ci, npm automatically runspretest:cifirst — that's how npm lifecycle scripts work, jive turkey! You be doublin' the build work for no reason!Remove the explicit build step and let
pretest:cihandle it. Your CI gonna move faster, I guarantee it!🧹 Remove the redundant build step
- - name: Build generated files needed for tests - run: npm run build:milestones && npm run build:js - - name: Run type check run: npm run typecheck🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/unit-tests.yml around lines 51 - 58, The workflow currently runs the build twice: the explicit step named "Build generated files needed for tests" runs npm run build:milestones && npm run build:js, and then the "Run tests with coverage" step runs npm run test:ci which already triggers pretest:ci (containing the same build chain). Remove the entire explicit build step (the job step that runs npm run build:milestones && npm run build:js) so tests rely on npm lifecycle pretest:ci, keeping the "Run tests with coverage" step that runs npm run test:ci unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In @.github/workflows/unit-tests.yml:
- Around line 51-58: The workflow currently runs the build twice: the explicit
step named "Build generated files needed for tests" runs npm run
build:milestones && npm run build:js, and then the "Run tests with coverage"
step runs npm run test:ci which already triggers pretest:ci (containing the same
build chain). Remove the entire explicit build step (the job step that runs npm
run build:milestones && npm run build:js) so tests rely on npm lifecycle
pretest:ci, keeping the "Run tests with coverage" step that runs npm run test:ci
unchanged.
In `@package.json`:
- Around line 8-10: The pretest build chain is duplicated in the "pretest" and
"pretest:ci" scripts; add a single shared script (e.g., "pretest:build") with
the command "npm run build:milestones && npm run build:js", then update both
"pretest" and "pretest:ci" to call "npm run pretest:build" so the build chain is
centralized and cannot drift.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro Plus
Run ID: ed5930d3-4c50-480e-a1ab-6c88f9368743
📒 Files selected for processing (2)
.github/workflows/unit-tests.ymlpackage.json
This was referenced Apr 26, 2026
This was referenced Apr 26, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
milestones-data.jsandscript.jsare now gitignored (generated files) but tests depend on thempretestandpretest:cilifecycle scripts topackage.jsonso bothnpm testandnpm run test:ciauto-build the needed filesnpm run build:milestones && npm run build:jsstep inunit-tests.ymlbeforenpm run test:ciSummary by CodeRabbit
Release Notes