Skip to content

Update Agents.md to run tests before assemble#5422

Open
zibet27 wants to merge 1 commit intomainfrom
zibet27/improve-agents-md
Open

Update Agents.md to run tests before assemble#5422
zibet27 wants to merge 1 commit intomainfrom
zibet27/improve-agents-md

Conversation

@zibet27
Copy link
Collaborator

@zibet27 zibet27 commented Mar 4, 2026

Motivation
Agents first run assemble and then only tests which use more tokens and take more time

Solution

  • Mention to run tests first

@zibet27 zibet27 requested review from e5l and osipxd March 4, 2026 10:40
@zibet27 zibet27 self-assigned this Mar 4, 2026
@coderabbitai
Copy link
Contributor

coderabbitai bot commented Mar 4, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 366f97f9-037a-46fe-8b7d-94280c02d1af

📥 Commits

Reviewing files that changed from the base of the PR and between 975ea8d and abdcd07.

📒 Files selected for processing (1)
  • AGENTS.md

Walkthrough

The AGENTS.md documentation file was updated to reorder workflow execution steps, placing test execution before compilation. A new guiding principle stating "Prefer this order to fail fast" was added after the ABI validation step, with minor spacing adjustments.

Changes

Cohort / File(s) Summary
Workflow step reordering
AGENTS.md
Reordered critical workflow steps to execute tests before compilation. Added new guiding line "Prefer this order to fail fast" after ABI validation step. Minor formatting/spacing adjustments applied.

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~3 minutes

Possibly related PRs

Suggested reviewers

  • e5l
🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately summarizes the main change: reordering workflow steps to run tests before assemble in the Agents.md documentation.
Description check ✅ Passed The description includes Motivation and Solution sections as required by the template but omits the Subsystem section.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch zibet27/improve-agents-md

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

1. Tests pass: `./gradlew :module-name:jvmTest` (and other platforms if touched)
2. Code compiles: `./gradlew :module-name:assemble`
3. Code is formatted and linting passes: run `./gradlew :module-name:formatKotlin`, then `./gradlew :module-name:lintKotlin`
4. ABI validated: `./gradlew :module-name:updateLegacyAbi` (if public/protected API changed)
Copy link
Member

Choose a reason for hiding this comment

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

We could ask it to use a one-liner (after the list):

./gradlew :module-name:assemble :module-name:<target>Test :module-name:formatKotlin :module-name:lintKotlin

What do you think?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I think we can ask it to first run tests and then use a one-liner for assemble, format, and lint

If we use only one one-liner and an error occurs, I guess it will try to rerun all tasks sequentially
Though it's just a guess, and I'll try to experiment with that

Copy link
Member

@osipxd osipxd Mar 4, 2026

Choose a reason for hiding this comment

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

By the way we could add a general rule that when agent needs to run multiple Gradle tasks it should pass all these tasks as arguments to a single ./gradlew command.

Copy link
Member

@e5l e5l left a comment

Choose a reason for hiding this comment

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

lgtm

@osipxd
Copy link
Member

osipxd commented Mar 5, 2026

Meh. CI needs to be started manually as we have unconditional requirement for checks, but this PR contains only *.md changes, so build hasn't started.

@e5l e5l enabled auto-merge (squash) March 9, 2026 06:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants