Skip to content

🤖 Add code review and function declaration/expression preference to AGENTS.md #7577

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
DrJKL merged 1 commit into from
Dec 17, 2025
Merged

🤖 Add code review and function declaration/expression preference to AGENTS.md #7577

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 25 additions & 1 deletion AGENTS.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -150,7 +150,8 @@ The project uses **Nx** for build orchestration and task management
21. Minimize [nesting](https://wiki.c2.com/?ArrowAntiPattern), e.g. `if () { ... }` or `for () { ... }`
22. Avoid mutable state, prefer immutability and assignment at point of declaration
23. Favor pure functions (especially testable ones)
24. Watch out for [Code Smells](https://wiki.c2.com/?CodeSmell) and refactor to avoid them
24. Do not use function expressions if it's possible to use function declarations instead
Copy link
Contributor

@christian-byrne christian-byrne Dec 17, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is that a backtick or apostrophe? GitHub is rendering this line weirdly

Copy link
Contributor Author

@DrJKL DrJKL Dec 17, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

', GitHub is special

25. Watch out for [Code Smells](https://wiki.c2.com/?CodeSmell) and refactor to avoid them

## Testing Guidelines

Expand Down Expand Up @@ -214,6 +215,29 @@ The project uses **Nx** for build orchestration and task management
- Thousands of users and extensions
- Prioritize clean interfaces that restrict extension access

### Code Review

In doing a code review, you should make sure that:

- The code is well-designed.
- The functionality is good for the users of the code.
- Any UI changes are sensible and look good.
- Any parallel programming is done safely.
- The code isn’t more complex than it needs to be.
- The developer isn’t implementing things they might need in the future but don’t know they need now.
- Code has appropriate unit tests.
- Tests are well-designed.
- The developer used clear names for everything.
- Comments are clear and useful, and mostly explain why instead of what.
- Code is appropriately documented (generally in g3doc).
- The code conforms to our style guides.

#### [Complexity](https://google.github.io/eng-practices/review/reviewer/looking-for.html#complexity)

Is the CL more complex than it should be? Check this at every level of the CL—are individual lines too complex? Are functions too complex? Are classes too complex? “Too complex” usually means “can’t be understood quickly by code readers.” It can also mean “developers are likely to introduce bugs when they try to call or modify this code.”

A particular type of complexity is over-engineering, where developers have made the code more generic than it needs to be, or added functionality that isn’t presently needed by the system. Reviewers should be especially vigilant about over-engineering. Encourage developers to solve the problem they know needs to be solved now, not the problem that the developer speculates might need to be solved in the future. The future problem should be solved once it arrives and you can see its actual shape and requirements in the physical universe.
Comment on lines +235 to +239
Copy link
Contributor

@coderabbitai coderabbitai bot Dec 17, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick | 🔵 Trivial

Polish the prose to reduce repetitive phrasing.

Lines 238–239 repeat "solve/solved" four times in close succession. While the guidance content is sound, consider rewording to improve readability.

- Encourage developers to solve the problem they know needs to be solved now, not the problem that the developer speculates might need to be solved in the future. The future problem should be solved once it arrives and you can see its actual shape and requirements in the physical universe.
+ Encourage developers to address the immediate problem they know needs solving now, rather than speculate about future problems. When a new problem actually emerges, they can see its concrete shape and requirements before solving it.

Based on static analysis hint.

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
#### [Complexity](https://google.github.io/eng-practices/review/reviewer/looking-for.html#complexity)
Is the CL more complex than it should be? Check this at every level of the CL—are individual lines too complex? Are functions too complex? Are classes too complex? “Too complex” usually means “can’t be understood quickly by code readers.” It can also mean “developers are likely to introduce bugs when they try to call or modify this code.”
A particular type of complexity is over-engineering, where developers have made the code more generic than it needs to be, or added functionality that isn’t presently needed by the system. Reviewers should be especially vigilant about over-engineering. Encourage developers to solve the problem they know needs to be solved now, not the problem that the developer speculates might need to be solved in the future. The future problem should be solved once it arrives and you can see its actual shape and requirements in the physical universe.
#### [Complexity](https://google.github.io/eng-practices/review/reviewer/looking-for.html#complexity)
Is the CL more complex than it should be? Check this at every level of the CL—are individual lines too complex? Are functions too complex? Are classes too complex? "Too complex" usually means "can't be understood quickly by code readers." It can also mean "developers are likely to introduce bugs when they try to call or modify this code."
A particular type of complexity is over-engineering, where developers have made the code more generic than it needs to be, or added functionality that isn't presently needed by the system. Reviewers should be especially vigilant about over-engineering. Encourage developers to address the immediate problem they know needs solving now, rather than speculate about future problems. When a new problem actually emerges, they can see its concrete shape and requirements before solving it.
🧰 Tools
🪛 LanguageTool

[style] ~239-~239: You have already used this phrasing in nearby sentences. Consider replacing it to add variety to your writing.
Context: ...lem that the developer speculates might need to be solved in the future. The future pro...

(REP_NEED_TO_VB)

🤖 Prompt for AI Agents 
In AGENTS.md around lines 235 to 239, the prose repeats "solve/solved" multiple
times in close succession, hurting readability; reword the passage to remove the
repeated phrasing by using a synonym or restructuring the sentence (e.g.,
replace one or two instances with words like "address", "resolve", or "implement
later", or combine clauses to avoid repetition), ensuring the original meaning
about avoiding over-engineering and favouring current solutions remains intact.

Copy link
Contributor Author

@DrJKL DrJKL Dec 17, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It was pulled verbatim.

Copy link
Contributor

@coderabbitai coderabbitai bot Dec 17, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@DrJKL, understood! Since the text is quoted directly from the source, it makes sense to maintain the original wording. Thank you for clarifying.

🧠 Learnings used 
Learnt from: christian-byrne
Repo: Comfy-Org/ComfyUI_frontend PR: 7358
File: src/components/dialog/content/signin/SignUpForm.vue:45-54
Timestamp: 2025-12-11T12:25:24.164Z
Learning: The project has CI automation that handles code formatting (pnpm format) automatically, so manual formatting suggestions should not be provided in code reviews for the Comfy-Org/ComfyUI_frontend repository.

Learnt from: DrJKL
Repo: Comfy-Org/ComfyUI_frontend PR: 7537
File: src/components/ui/button/Button.stories.ts:45-55
Timestamp: 2025-12-17T00:40:09.635Z
Learning: In the Comfy-Org/ComfyUI_frontend repository, prefer pure function declarations (e.g., `function foo() { ... }`) over function expressions (e.g., `const foo = () => { ... }`) for pure functions, as function declarations are considered more functional than function expressions.


## Repository Navigation

- Check README files in key folders (tests-ui, browser_tests, composables, etc.)
Expand Down