Pain points
1. Don't commit files with lint
Problem: Agents sometimes fail to fix linting errors. These are later caught by automated tests, causing extra work to fix them and creating a disconnect between a) the code in the agents' commits and b) the final code submitted in the PR.
An easy solution is to run the "fix lint" command (which is pnpm run lint in virtually all repos) to fix lint automatically, and then pnpm run lint:strict (if it exists) to verify that no lint remains. But the output adds messages to the context window.
Proposed solution: Modify the lint and lint:strict commands to accept an --agent option that silences all output except for errors.
2. Obey commit conventions
Commit title: No more than 72 characters. Use the {workspace}|{work type}: prefix (if in a monorepo) or work type: prefix otherwise. Don't include the ticket ID in the commit title.
Commit body:
- Add a period, comma, or semicolon to the end of bulleted list items, to make them easier to understand when rendered without line breaks.
- Break up large paragraphs into smaller ones. Add a blank line between paragraphs.
- Punctuate bulleted items so they can be understood when concatenated: Each item should end in a comma, semicolon, or period.
- Use backticks in the same way they would be used in a PR description. Examples:
- "Adds a
refresh method to MyClass."
- "Makes the
variables parameter of fetchResults optional."
- "Upgrades all ESLint packages to the latest version; exception: pinned
eslint-plugin-import to v2.32.0 to avoid breaking changes."
- Don't include the ticket ID in the commit title. This was an old convention. If a skill specifies that the ticket ID should be included in the commit title, propose a fix to the skill. Ticket IDs are included when the PR is merged, not when commits are creeated.
3. Use informative variable names
Avoid abbreviations:
- Incorrect:
pos, req, res. Correct: position, request, response.
- An exception can be made for predicate functions where the abbreviation doesn't impede understanding. OK:
.map(c => c.trim()).
If a variable stores a unit of measure, always indicate the unit of measure in the name of the variable. Abbreviations are OK if clearly understandable: Incorrect: duration, CELL_HEIGHT; correct: durationMs, CELL_HEIGHT_PX.
Function names should start with a transitive verb.
Boolean names should start with
should oris, has, or does (in the appropriate conjugation: "was", "are", "have", "did", etc.).
4. Eliminate noisy references to automated tests
Never include any automated checks in the test plan for PR. Never refer to automated tests in commit messages. These checks, which run automatically in the CI/CD pipeline when a PR is created, are automated checks:. formatting, linting, unit tests, and typechecking.
5. Don't use review finding IDs out of context.
Finding IDs such as F1, T1, W2, S3, etc. make sense only in the context of a review document.
Don't use these IDs in any other context, such as commit messages, plans, tickets, and PR descriptions.
When posting review findings as comments on a PR, do not refer to the finding ID. Prepend a label to the comment instead. Incorrect: F1. Some comment; correct: fixme: F1. Some comment. F -> 'fixme:', W -> 'warning:', T -> 'todo:', S -> 'suggestion:', R -> 'recommendation:'.
Pain points
1. Don't commit files with lint
Problem: Agents sometimes fail to fix linting errors. These are later caught by automated tests, causing extra work to fix them and creating a disconnect between a) the code in the agents' commits and b) the final code submitted in the PR.
An easy solution is to run the "fix lint" command (which is
pnpm run lintin virtually all repos) to fix lint automatically, and thenpnpm run lint:strict(if it exists) to verify that no lint remains. But the output adds messages to the context window.Proposed solution: Modify the
lintandlint:strictcommands to accept an--agentoption that silences all output except for errors.2. Obey commit conventions
Commit title: No more than 72 characters. Use the
{workspace}|{work type}:prefix (if in a monorepo) orwork type:prefix otherwise. Don't include the ticket ID in the commit title.Commit body:
refreshmethod toMyClass."variablesparameter offetchResultsoptional."eslint-plugin-importto v2.32.0 to avoid breaking changes."3. Use informative variable names
Avoid abbreviations:
pos,req,res. Correct:position,request,response..map(c => c.trim()).If a variable stores a unit of measure, always indicate the unit of measure in the name of the variable. Abbreviations are OK if clearly understandable: Incorrect:
duration,CELL_HEIGHT; correct:durationMs,CELL_HEIGHT_PX.Function names should start with a transitive verb.
Boolean names should start with
shouldoris,has, ordoes(in the appropriate conjugation: "was", "are", "have", "did", etc.).4. Eliminate noisy references to automated tests
Never include any automated checks in the test plan for PR. Never refer to automated tests in commit messages. These checks, which run automatically in the CI/CD pipeline when a PR is created, are automated checks:. formatting, linting, unit tests, and typechecking.
5. Don't use review finding IDs out of context.
Finding IDs such as F1, T1, W2, S3, etc. make sense only in the context of a review document.
Don't use these IDs in any other context, such as commit messages, plans, tickets, and PR descriptions.
When posting review findings as comments on a PR, do not refer to the finding ID. Prepend a label to the comment instead. Incorrect:
F1. Some comment; correct:fixme: F1. Some comment. F -> 'fixme:', W -> 'warning:', T -> 'todo:', S -> 'suggestion:', R -> 'recommendation:'.