chore: Maintenance updates#143
Conversation
Signed-off-by: Dave Skender <8432125+DaveSkender@users.noreply.github.com>
📝 WalkthroughWalkthroughThis pull request updates project tooling and configurations: adds ✨ Finishing Touches✨ Simplify code
Comment |
Signed-off-by: Dave Skender <8432125+DaveSkender@users.noreply.github.com>
Deploying uswds-design-patterns with
|
| Latest commit: |
927ccc3
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://8bb698fb.uswds-design-patterns.pages.dev |
| Branch Preview URL: | https://maintenance.uswds-design-patterns.pages.dev |
Updated task labels and details in tasks.json to reflect correct npm commands for managing Node packages. Signed-off-by: Dave Skender <8432125+DaveSkender@users.noreply.github.com>
Signed-off-by: Dave Skender <8432125+DaveSkender@users.noreply.github.com>
Expanded ignores in markdownlint configuration to exclude additional directories. Signed-off-by: Dave Skender <8432125+DaveSkender@users.noreply.github.com>
Signed-off-by: Dave Skender <8432125+DaveSkender@users.noreply.github.com>
Signed-off-by: Dave Skender <8432125+DaveSkender@users.noreply.github.com>
Signed-off-by: Dave Skender <8432125+DaveSkender@users.noreply.github.com>
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (2)
.markdownlint-cli2.jsonc (1)
4-13: Minor: Redundant ignore pattern.
"**/node_modules/**"already matchesnode_modulesat any depth including the root, making"node_modules/**"redundant. Consider removing line 5 for clarity.Suggested simplification
"ignores": [ - "node_modules/**", - "**/node_modules/**", + "**/node_modules/**", "dist/**",🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.markdownlint-cli2.jsonc around lines 4 - 13, In the "ignores" array remove the redundant "node_modules/**" entry and keep the more general "**/node_modules/**" pattern; locate the "ignores" key and delete the literal "node_modules/**" string so only "**/node_modules/**", "dist/**", "out/**", etc. remain to avoid duplicate patterns.package.json (1)
14-14: Optional:cleanscript is not cross-platform.
rm -rfandrm -fare Unix commands that won't work on native Windows command prompts. If Windows developer support is needed, consider a cross-platform alternative.Cross-platform alternative using rimraf
Add
rimrafas a dev dependency, then:- "clean": "rm -rf node_modules && rm -f pnpm-lock.yaml package-lock.json yarn.lock", + "clean": "rimraf node_modules pnpm-lock.yaml package-lock.json yarn.lock",Alternatively, if targeting npm 7+ only, the script could use
npx rimrafwithout adding the dependency.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@package.json` at line 14, The "clean" script in package.json uses Unix-only rm commands; make it cross-platform by replacing the rm usage in the "clean" script with a cross-platform remover (e.g., use rimraf for directories/files) and add rimraf as a devDependency (or use npx rimraf to avoid adding a dep); update the "clean" script to call rimraf for node_modules and the lock files so it works on Windows, macOS, and Linux while keeping the same intent.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.vscode/tasks.json:
- Line 12: The JSON arrays violate jsonc/array-bracket-newline: update the empty
"problemMatcher": [] to use multi-line brackets (open bracket on one line and
closing bracket on its own line) and likewise format the populated arrays at the
other locations so the opening '[' is followed by a newline and each element (or
the single element) is on its own line before the closing ']' — locate and edit
the "problemMatcher" array and the two other arrays shown in the diff to add the
required line breaks.
- Around line 71-87: The "detail" string for the task labeled "Lint: All
(verify)" mentions "ai + tests" but there is no corresponding AI lint task in
the dependsOn array; update the task by removing the stray "ai" reference from
the detail text (or alternatively add the appropriate AI lint task to dependsOn
if intended) so the description matches the dependsOn list for the "Lint: All
(verify)" task.
- Around line 62-70: The task entry labeled "Test" has a misleading detail
string saying "Run vitest test suite" while its "script" is "test" which runs
Jest; update the "detail" to accurately describe the task or change the "script"
to the vitest runner. Locate the task object with "label": "Test" and either
change its "detail" text to "Run Jest test suite" (or similar) to match the
"script": "test", or update the "script" to the vitest command (e.g., the vitest
npm script name) if the project has migrated and package.json provides a vitest
script.
- Around line 38-43: The "Update: Angular packages" task currently runs the
shell "command" containing "@angular/material" which is not in package.json;
remove "@angular/material" from the command string in the tasks.json entry
labeled "Update: Angular packages" so the command becomes "ng update
`@angular/cli` `@angular/core` angular-eslint --allow-dirty" to avoid update errors.
In `@package.json`:
- Around line 17-18: The package.json scripts "lint:md" and "lint:md:fix" call
npx markdownlint-cli2 but devDependencies currently list "markdownlint-cli";
update the devDependencies to install "markdownlint-cli2" (replace the
"markdownlint-cli" entry with "markdownlint-cli2", selecting a compatible
version) so the scripts succeed and align with the new .markdownlint-cli2.jsonc
configuration.
---
Nitpick comments:
In @.markdownlint-cli2.jsonc:
- Around line 4-13: In the "ignores" array remove the redundant
"node_modules/**" entry and keep the more general "**/node_modules/**" pattern;
locate the "ignores" key and delete the literal "node_modules/**" string so only
"**/node_modules/**", "dist/**", "out/**", etc. remain to avoid duplicate
patterns.
In `@package.json`:
- Line 14: The "clean" script in package.json uses Unix-only rm commands; make
it cross-platform by replacing the rm usage in the "clean" script with a
cross-platform remover (e.g., use rimraf for directories/files) and add rimraf
as a devDependency (or use npx rimraf to avoid adding a dep); update the "clean"
script to call rimraf for node_modules and the lock files so it works on
Windows, macOS, and Linux while keeping the same intent.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository: facioquo/coderabbit/.coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 365c4bba-5d49-4c1a-91de-d364ff555558
📒 Files selected for processing (5)
.markdownlint-cli2.jsonc.markdownlint.json.markdownlintignore.vscode/tasks.jsonpackage.json
💤 Files with no reviewable changes (2)
- .markdownlintignore
- .markdownlint.json
Signed-off-by: Dave Skender <8432125+DaveSkender@users.noreply.github.com>
Signed-off-by: Dave Skender <8432125+DaveSkender@users.noreply.github.com>
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
package.json (1)
18-19:⚠️ Potential issue | 🟠 Major
markdownlint-cli2is missing fromdevDependencies.The scripts
lint:mdandlint:md:fixinvokenpx markdownlint-cli2, but the package is not listed indevDependencies. Whilenpxwill download it on-demand, this makes builds slower and less deterministic. The package should be explicitly installed.Proposed fix
"jsonc-eslint-parser": "^3.1.0", + "markdownlint-cli2": "^0.22.0", "prettier": "^3.8.2",🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@package.json` around lines 18 - 19, The package.json scripts "lint:md" and "lint:md:fix" call npx markdownlint-cli2 but markdownlint-cli2 is not listed in devDependencies; add "markdownlint-cli2" to devDependencies (with a stable version) and run npm install (or yarn) so the linter is installed deterministically rather than fetched via npx at runtime.
🧹 Nitpick comments (1)
setup-vitest.ts (1)
40-41: Prefer enabling Angular teardown to improve test isolation.Setting
destroyAfterEach: falsecan leak component/provider state across specs and make failures order-dependent. Consider defaulting totrueunless there’s a measured reason not to.Suggested update
getTestBed().initTestEnvironment(BrowserDynamicTestingModule, platformBrowserDynamicTesting(), { - teardown: { destroyAfterEach: false, rethrowErrors: true } + teardown: { destroyAfterEach: true, rethrowErrors: true } });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@setup-vitest.ts` around lines 40 - 41, The initTestEnvironment call currently disables Angular teardown by setting teardown.destroyAfterEach to false; change the teardown option in getTestBed().initTestEnvironment(BrowserDynamicTestingModule, platformBrowserDynamicTesting(), { ... }) to set destroyAfterEach: true (leave rethrowErrors: true) so each spec is destroyed after run and tests are isolated (adjust only the teardown.destroyAfterEach flag in that call).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@package.json`:
- Around line 35-36: CI workflows are still using Node 22; update the GitHub
workflow steps that use actions/setup-node to specify node-version: "24" so they
align with package.json's engines field. Edit .github/workflows/test-website.yml
and .github/workflows/deploy-website.yml and change any occurrences of
node-version or matrix entries targeting 22 to "24" (look for the
actions/setup-node step and keys named node-version or the matrix.node entries)
and ensure the workflows pass Node 24 in all relevant job definitions.
---
Duplicate comments:
In `@package.json`:
- Around line 18-19: The package.json scripts "lint:md" and "lint:md:fix" call
npx markdownlint-cli2 but markdownlint-cli2 is not listed in devDependencies;
add "markdownlint-cli2" to devDependencies (with a stable version) and run npm
install (or yarn) so the linter is installed deterministically rather than
fetched via npx at runtime.
---
Nitpick comments:
In `@setup-vitest.ts`:
- Around line 40-41: The initTestEnvironment call currently disables Angular
teardown by setting teardown.destroyAfterEach to false; change the teardown
option in getTestBed().initTestEnvironment(BrowserDynamicTestingModule,
platformBrowserDynamicTesting(), { ... }) to set destroyAfterEach: true (leave
rethrowErrors: true) so each spec is destroyed after run and tests are isolated
(adjust only the teardown.destroyAfterEach flag in that call).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository: facioquo/coderabbit/.coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: e9d7e352-4b9a-4de6-952c-8fda17b10651
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json,!*-lock.jsonand included by**
📒 Files selected for processing (47)
.github/instructions/code-completion.instructions.md.markdownlint-cli2.jsonc.prettierignore.prettierrc.json.vscode/mcp.json.vscode/tasks.jsonREADME.mdeslint.config.jsjest.config.tspackage.jsonplaywright.config.tssass/_uswds-base-settings.scsssetup-jest.tssetup-vitest.tssrc/app/app-routing.module.spec.tssrc/app/app.component.spec.tssrc/app/app.component.tssrc/app/components/pattern-footer/pattern-footer.component.tssrc/app/components/pattern-header/pattern-header.component.tssrc/app/components/pattern-settings/pattern-settings.component.tssrc/app/components/site-banner/site-banner.component.tssrc/app/components/site-card/site-card.component.tssrc/app/components/site-identifier/site-identifier.component.tssrc/app/pages/404/404.component.spec.tssrc/app/pages/404/404.component.tssrc/app/pages/__tests__/seo-resolvers.spec.tssrc/app/pages/bubbles/bubbles.component.spec.tssrc/app/pages/bubbles/bubbles.component.tssrc/app/pages/card-catalog/card-catalog.component.spec.tssrc/app/pages/card-catalog/card-catalog.component.tssrc/app/pages/hero-overlay/hero-overlay.component.spec.tssrc/app/pages/hero-overlay/hero-overlay.component.tssrc/app/pages/home/home.component.spec.tssrc/app/pages/home/home.component.tssrc/app/pages/image.model.tssrc/app/pages/patterns.model.tssrc/app/pages/sticky-menu/sticky-menu.component.spec.tssrc/app/pages/sticky-menu/sticky-menu.component.tssrc/app/services/__tests__/app-title.strategy.spec.tssrc/app/services/__tests__/utility.service.spec.tssrc/app/services/utility.model.tssrc/app/services/utility.service.tssrc/index.htmlsrc/main.tstsconfig.jsontsconfig.spec.jsonvitest.config.ts
💤 Files with no reviewable changes (3)
- .prettierignore
- setup-jest.ts
- jest.config.ts
✅ Files skipped from review due to trivial changes (33)
- src/app/components/pattern-footer/pattern-footer.component.ts
- src/app/components/site-card/site-card.component.ts
- src/app/components/site-identifier/site-identifier.component.ts
- src/app/app.component.spec.ts
- src/app/components/site-banner/site-banner.component.ts
- src/app/pages/home/home.component.spec.ts
- src/app/pages/sticky-menu/sticky-menu.component.ts
- src/app/components/pattern-header/pattern-header.component.ts
- src/app/components/pattern-settings/pattern-settings.component.ts
- sass/_uswds-base-settings.scss
- src/app/pages/404/404.component.ts
- .prettierrc.json
- src/index.html
- src/app/pages/home/home.component.ts
- src/app/pages/404/404.component.spec.ts
- src/app/pages/hero-overlay/hero-overlay.component.spec.ts
- src/app/services/utility.service.ts
- README.md
- src/app/app.component.ts
- .vscode/mcp.json
- src/app/pages/bubbles/bubbles.component.ts
- tsconfig.spec.json
- playwright.config.ts
- src/app/pages/tests/seo-resolvers.spec.ts
- src/app/services/utility.model.ts
- src/main.ts
- src/app/pages/card-catalog/card-catalog.component.ts
- src/app/pages/patterns.model.ts
- .github/instructions/code-completion.instructions.md
- src/app/app-routing.module.spec.ts
- src/app/pages/hero-overlay/hero-overlay.component.ts
- .markdownlint-cli2.jsonc
- src/app/pages/sticky-menu/sticky-menu.component.spec.ts
|
@copilot fix all reasons for ci check failures, and resolve all open pr review feedback. Do self-review before finishing, including code completion checklist. |
… dep, sync package-lock.json Agent-Logs-Url: https://github.com/facioquo/uswds-design-patterns/sessions/ef08944a-9ff6-4ca9-adaa-9a3cb89bab63 Co-authored-by: DaveSkender <8432125+DaveSkender@users.noreply.github.com>
Fixed all CI failures and open review feedback in commit 273fb0e:
All checks pass locally: lint ✅ build ✅ unit tests ✅ |
…est isolation Agent-Logs-Url: https://github.com/facioquo/uswds-design-patterns/sessions/8933f175-e811-4ab5-9cc4-c017c9e7dcc8 Co-authored-by: DaveSkender <8432125+DaveSkender@users.noreply.github.com>
|
@copilot fix all Codacy issues and and Angular modernizations, use MCP servers for both. |
Agent-Logs-Url: https://github.com/facioquo/uswds-design-patterns/sessions/652de9fc-8fc9-4c3f-9421-39d29abf5d07 Co-authored-by: DaveSkender <8432125+DaveSkender@users.noreply.github.com>
Fixed in commit 927ccc3. Codacy issues (via Codacy MCP — all 8 new issues in
Angular modernizations (via Angular CLI MCP best practices for v21):
Lint ✅ Build ✅ Tests 14/14 ✅ |
Changes: