chore(copilot-instructions): add the copilot-instructions.md#7926
chore(copilot-instructions): add the copilot-instructions.md#7926nineteen88 wants to merge 1 commit intomasterfrom
Conversation
There was a problem hiding this comment.
Pull request overview
Adds a repository-specific Copilot agent instruction document under .github/ to guide automated changes/reviews in the carbon-react codebase.
Changes:
- Introduces
.github/copilot-instructions.mddescribing repo tooling, workflows, project layout, and conventions. - Documents local bootstrap commands and CI-equivalent validation steps.
- Captures testing conventions (Jest/RTL + Playwright CT) and “skills sync” expectations.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
includes information for copilot agents
nineteen88
force-pushed
the
add-copilot-instructions
branch
from
b42f0a9 to
e37fad2
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 1 out of 1 changed files in this pull request and generated 3 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
|
||
| Key conventions (follow them — CI/lint will enforce most): | ||
|
|
||
| - Use `data-role` (NOT `data-testid`) for test queries; `data-component` is reserved for component roots, `data-element` for sub-elements. Both Jest (RTL) and Playwright are configured to read `data-role` as the test-id attribute. |
There was a problem hiding this comment.
issue: The doc says to use data-role (NOT data-testid) for test queries, but the repo still contains data-testid usage (e.g. in Storybook interaction stories). Consider clarifying this as “prefer data-role for tests (RTL/Playwright are configured for it)”, and note any exceptions (Storybook/pseudo state selectors, legacy code) to avoid confusing contributors/agents.
| - Use `data-role` (NOT `data-testid`) for test queries; `data-component` is reserved for component roots, `data-element` for sub-elements. Both Jest (RTL) and Playwright are configured to read `data-role` as the test-id attribute. | |
| - Prefer `data-role` for test queries; `data-component` is reserved for component roots, `data-element` for sub-elements. Both Jest (RTL) and Playwright are configured to read `data-role` as the test-id attribute. `data-testid` may still appear in Storybook-specific selectors/pseudo states or legacy code, so treat `data-role` as the default for new tests rather than an absolute ban. |
| | Type check | `npm run type-check` | `tsc --noEmit` against [tsconfig.json](../tsconfig.json). | | ||
| | Skills sync | `npm run build:skills -- --check` | Fails if files in [skills/](../skills) are stale relative to component changes. Regenerate with `npm run build:skills` and commit the diff. | |
There was a problem hiding this comment.
issue: The validation pipeline lists npm run type-check as a standalone step, but that script does not run token generation; without a prior generate-tokens:dev run (or running lint/test first) it can fail due to missing static-tokens imports. Suggest updating this step (or §4) to mention the required token generation order, or adjusting the type-check script to run generate-tokens:dev first.
| | Step | Command | Notes | | ||
| |---|---|---| | ||
| | Format check | `npx prettier --check './src/**/*.{js,jsx,ts,tsx}'` | Auto-fix with `npm run format`. | | ||
| | Lint | `npm run lint` | ESLint flat config in [eslint.config.mjs](../eslint.config.mjs). Uses `--max-warnings=636` and `--report-unused-disable-directives`; do not introduce new warnings or unused `eslint-disable` lines. | |
There was a problem hiding this comment.
nitpick: The lint step note implies CI prevents new warnings, but --max-warnings=636 only caps the total warnings. Consider rephrasing to “CI allows up to 636 warnings; avoid increasing the count / fix warnings rather than raising the ceiling” for accuracy.
| | Lint | `npm run lint` | ESLint flat config in [eslint.config.mjs](../eslint.config.mjs). Uses `--max-warnings=636` and `--report-unused-disable-directives`; do not introduce new warnings or unused `eslint-disable` lines. | | |
| | Lint | `npm run lint` | ESLint flat config in [eslint.config.mjs](../eslint.config.mjs). CI allows up to 636 warnings; avoid increasing the count, and fix warnings rather than raising the ceiling. Also uses `--report-unused-disable-directives`, so do not introduce unused `eslint-disable` lines. | |
tomdavies73
left a comment
tomdavies73
left a comment
There was a problem hiding this comment.
@nineteen88 might be worth seeing if we can ignore the skills.md files as I've noticed github co-pilot commenting on these skills changes without the context or nuance around said changes
Proposed behaviour
Add the file with information that will be consumed by copilot agents
Current behaviour
No copilot-instructions.md file currently
Checklist
d.tsfile added or updated if requiredQA
Additional context
Testing instructions