Skip to content

[hotfix][docs] Add code review guidelines to AGENTS.md - #28861

Merged
rkhachatryan merged 1 commit into
apache:masterfrom
rkhachatryan:hotfix-agents-review-suggestions
Aug 4, 2026
Merged

[hotfix][docs] Add code review guidelines to AGENTS.md#28861
rkhachatryan merged 1 commit into
apache:masterfrom
rkhachatryan:hotfix-agents-review-suggestions

Conversation

@rkhachatryan

Copy link
Copy Markdown
Contributor

No description provided.

@flinkbot

flinkbot commented Jul 31, 2026

Copy link
Copy Markdown
Collaborator

CI report:

Bot commands The @flinkbot bot supports the following commands:
  • @flinkbot run azure re-run the last Azure build

@Efrat19 Efrat19 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM

@github-actions github-actions Bot added the community-reviewed PR has been reviewed by the community. label Aug 2, 2026
@rkhachatryan
rkhachatryan force-pushed the hotfix-agents-review-suggestions branch from e505542 to 7338eab Compare August 3, 2026 09:10

@Izeren Izeren left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thank you for the change, @rkhachatryan

Comment thread AGENTS.md Outdated

- Look for opportunities to simplify the code, scoped to the diff itself (not pre-existing code outside the change).
- Flag comments that are obvious (restate what the code already says) or overly verbose.
- In test code, look for potential flakiness — e.g. `Thread.sleep` used outside a retry/poll loop, or similar timing-dependent, non-deterministic patterns.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

We could add something about suggestion to use clock injection for test determinism. This was suggestions from reviewer would be more helpful

@rkhachatryan rkhachatryan Aug 3, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Good idea!

Update the PR, also added a mention about waiting on a condition in a loop.

@github-actions github-actions Bot added the community-reviewed-LGTM Applied if there are 2 non-committer approves on a PR. (The submitter cannot approve their own PR.) label Aug 3, 2026
@rkhachatryan
rkhachatryan force-pushed the hotfix-agents-review-suggestions branch from 7338eab to 91fbece Compare August 3, 2026 21:41
@rkhachatryan
rkhachatryan marked this pull request as ready for review August 3, 2026 21:43
@rkhachatryan

Copy link
Copy Markdown
Contributor Author

Thanks for the reviews!
CI failures is unrelated, merging.

@rkhachatryan
rkhachatryan merged commit 3c8740f into apache:master Aug 4, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

community-reviewed PR has been reviewed by the community. community-reviewed-LGTM Applied if there are 2 non-committer approves on a PR. (The submitter cannot approve their own PR.)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants