Skip to content

Conversation

@teeohhem
Copy link
Contributor

No description provided.

@changeset-bot
Copy link

changeset-bot bot commented Oct 14, 2025

⚠️ No Changeset found

Latest commit: f38ca1d

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@vercel
Copy link

vercel bot commented Oct 14, 2025

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Preview Comments Updated (UTC)
hyperdx-v2-oss-app Ready Ready Preview Comment Oct 14, 2025 9:28pm

@claude
Copy link

claude bot commented Oct 14, 2025

PR Review: Update claude.md with more linting instructions

Summary

This PR updates the CLAUDE.md documentation to provide clearer linting instructions for Claude AI agents before committing code. The changes emphasize the need to manually run linting commands and handle any remaining issues that can't be auto-fixed.

✅ Strengths

  1. Improved Clarity: The new section title is much more prominent and attention-grabbing than the previous Lint Fix Commands section

  2. Better Context: The explanation of why Claude AI agents need to manually run linting (because pre-commit hooks don't trigger for agents) is valuable context

  3. Clear Process: The step-by-step instructions for handling linting issues that can't be auto-fixed are helpful

  4. Consistent Command Format: Using yarn run lint:fix is more explicit than the previous yarn lint:fix

🐛 Issues Found

1. Typo in PR Title (Minor)

  • PR title has instrucitons instead of instructions

2. Missing Package Order Consistency (Minor)

The order of packages changed from api, app, common-utils to app, api, common-utils. Recommend keeping a consistent order (suggest alphabetical: api, app, common-utils, or logical: common-utils, api, app since common-utils is a dependency)

3. Removed Useful NX Command (Moderate)

The previous version included npx nx run-many -t lint:fix which is actually a useful alternative that could save time by running lint:fix across all packages at once. Consider keeping this as an optional shortcut.

4. Inconsistent Command Format (Minor)

The last command (cd packages/common-utils && yarn lint:fix) doesn't use yarn run like the other two. Use consistent format for all three commands.

💡 Suggestions for Enhancement

  1. Add Example Output: Consider showing what linting errors look like so agents know what to expect

  2. Clarify Modified Packages: The comment says Fix linting issues in modified packages but then lists all three packages. Consider being more explicit about only running lint:fix for packages you've actually modified.

✅ Overall Assessment

This is a valuable improvement to the documentation that will help Claude AI agents better understand the linting requirements. The issues identified are minor and easily addressed.

Recommendation: ✅ Approve with minor suggestions

The core improvement (making linting instructions more prominent and explicit) is excellent. Consider addressing the typo in the PR title and the command consistency issues in a follow-up.

@github-actions
Copy link
Contributor

E2E Test Results

All tests passed • 25 passed • 3 skipped • 229s

Status Count
✅ Passed 25
❌ Failed 0
⚠️ Flaky 0
⏭️ Skipped 3

View full report →

@kodiakhq kodiakhq bot merged commit 79af32e into main Oct 14, 2025
10 checks passed
@kodiakhq kodiakhq bot deleted the tom/claude-md-linting branch October 14, 2025 21:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants