-
Notifications
You must be signed in to change notification settings - Fork 119
agents: Update rules #3717
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.
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
agents: Update rules #3717
Conversation
Signed-off-by: Patrick José Pereira <patrickelectric@gmail.com>
Reviewer's guide (collapsed on small PRs)Reviewer's GuideUpdates AGENTS.md contributor guidelines to prefer TypeScript optional chaining, Vue 2 v-tooltip usage, and yarn over npm in project workflows. File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey - I've found 2 issues, and left some high level feedback:
- The guidance to 'Use optional chaining (
?.) when possible in typescript' is a bit vague—consider clarifying when it should be avoided (e.g., where explicit null checks are important for logic or error handling). - For 'Use
v-tooltipovertitlein vue2 components', it may help to briefly mention the reasoning (e.g., consistency, styling, accessibility) and any exceptions wheretitleis still acceptable. - The new 'Use yarn over npm' section could be more actionable by specifying whether this applies to all scripts/CI tooling and, if relevant, which yarn version and lockfile strategy the project standardizes on.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The guidance to 'Use optional chaining (`?.`) when possible in typescript' is a bit vague—consider clarifying when it should be avoided (e.g., where explicit null checks are important for logic or error handling).
- For 'Use `v-tooltip` over `title` in vue2 components', it may help to briefly mention the reasoning (e.g., consistency, styling, accessibility) and any exceptions where `title` is still acceptable.
- The new 'Use yarn over npm' section could be more actionable by specifying whether this applies to all scripts/CI tooling and, if relevant, which yarn version and lockfile strategy the project standardizes on.
## Individual Comments
### Comment 1
<location> `AGENTS.md:49` </location>
<code_context>
- No docstrings unless the function is non-obvious
- No comments unless explaining "why", never "what"
- Prefer editing existing files over creating new ones
+- Use optional chaining (`?.`) when possible in typescript
+- Use `v-tooltip` over `title` in vue2 components
</code_context>
<issue_to_address>
**nitpick (typo):** Capitalize "TypeScript" as a proper noun.
Please update "typescript" to "TypeScript" for correct capitalization.
```suggestion
- Use optional chaining (`?.`) when possible in TypeScript
```
</issue_to_address>
### Comment 2
<location> `AGENTS.md:81` </location>
<code_context>
gh issue list --repo bluerobotics/BlueOS
```
+### 3. Use yarn over npm
+
## Creating a New Service
</code_context>
<issue_to_address>
**suggestion (typo):** Consider capitalizing "Yarn" as the tool name.
Also keep “npm” lowercase so both tool names match their official styling.
```suggestion
### 3. Use Yarn over npm
```
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| - No docstrings unless the function is non-obvious | ||
| - No comments unless explaining "why", never "what" | ||
| - Prefer editing existing files over creating new ones | ||
| - Use optional chaining (`?.`) when possible in typescript |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nitpick (typo): Capitalize "TypeScript" as a proper noun.
Please update "typescript" to "TypeScript" for correct capitalization.
| - Use optional chaining (`?.`) when possible in typescript | |
| - Use optional chaining (`?.`) when possible in TypeScript |
| gh issue list --repo bluerobotics/BlueOS | ||
| ``` | ||
|
|
||
| ### 3. Use yarn over npm |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
suggestion (typo): Consider capitalizing "Yarn" as the tool name.
Also keep “npm” lowercase so both tool names match their official styling.
| ### 3. Use yarn over npm | |
| ### 3. Use Yarn over npm |
Summary by Sourcery
Update internal agent guidelines and tooling preferences.
Documentation: