Skip to content

dnm: validation#382

Closed
andrew-anyscale wants to merge 3 commits intoandrew/revup/main/matrix-converterfrom
andrew/revup/main/matrix-testedd
Closed

dnm: validation#382
andrew-anyscale wants to merge 3 commits intoandrew/revup/main/matrix-converterfrom
andrew/revup/main/matrix-testedd

Conversation

@andrew-anyscale
Copy link
Contributor

Relative: matrix-converter
Topic: matrix-testedd
Labels: draft
Signed-off-by: andrew andrew@anyscale.com

Rayci will expand matrix into individual Buildkite steps, allowing for fine-grained depends_on targeting of specific matrix combinations.

Topic: matrix-expand
Labels: draft
Signed-off-by: andrew <andrew@anyscale.com>
Integrate matrix_expand into conversion steps. This allows pipeline steps to selectively depend on a subset of matrix-defined
steps, reducing unnecessary steps when tag filtering can filter those out.

- Introduced a new YAML configuration for matrix selection tests in Buildkite.
- Implemented matrix expansion logic in the converter, allowing for dynamic step generation based on matrix configurations.

Topic: matrix-converter
Relative: matrix-expand
Labels: draft

Signed-off-by: andrew <andrew@anyscale.com>
Relative: matrix-converter
Topic: matrix-testedd
Labels: draft
Signed-off-by: andrew <andrew@anyscale.com>
@andrew-anyscale
Copy link
Contributor Author

Reviews in this chain:
#378 feat(raycicmd): add matrix_expand
 └#379 feat(raycicmd): add matrix expansion and filtering support
  └#382 dnm: validation

@andrew-anyscale
Copy link
Contributor Author

andrew-anyscale commented Jan 20, 2026

# head base diff date summary
0 af28d5b0 feac46f5 diff Jan 20 14:50 PM 1 file changed, 1 insertion(+), 1 deletion(-)
1 6e29f198 4c8d9d26 rebase Jan 20 14:55 PM 0 files changed
2 ef5cae9b 83f7a551 rebase Jan 26 11:22 AM 0 files changed
3 5e3f8d0b 2d792f3d rebase Jan 26 11:32 AM 0 files changed
4 fa612a45 4214f2b7 rebase Jan 26 11:42 AM 0 files changed

@gemini-code-assist
Copy link
Contributor

Summary of Changes

Hello @andrew-anyscale, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

This pull request introduces a minor cleanup within a test file, specifically removing a redundant comment. While the title suggests a broader 'validation' effort, the current changes are limited to this small refinement, potentially laying groundwork or being an isolated part of a larger, ongoing development.

Highlights

  • Test File Cleanup: Removed an unnecessary comment // Saturday from a test case definition in wanda/cache_epoch_test.go.

🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console.

Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point by creating a comment using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.

Feature Command Description
Code Review /gemini review Performs a code review for the current pull request in its current state.
Pull Request Summary /gemini summary Provides a summary of the current pull request in its current state.
Comment @gemini-code-assist Responds in comments when explicitly tagged, both in pull request comments and review comments.
Help /gemini help Displays a list of available commands.

Customization

To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here.

You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.

Footnotes

  1. Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution.

Copy link
Contributor

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request makes a minor change to a test file by removing a comment. My review suggests restoring this comment to maintain consistency and readability within the test suite, as other test cases include similar comments for clarity.

}{{
name: "saturday",
now: morningOf(2025, time.May, 31), // Saturday
now: morningOf(2025, time.May, 31),
Copy link
Contributor

Choose a reason for hiding this comment

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

medium

This change removes the comment indicating the day of the week for this test case. For consistency with the other test cases in this file, which all have such comments, it's recommended to restore it. This improves readability and makes it easier to verify the test cases at a glance.

Suggested change
now: morningOf(2025, time.May, 31),
now: morningOf(2025, time.May, 31), // Saturday

@andrew-anyscale andrew-anyscale force-pushed the andrew/revup/main/matrix-testedd branch from af28d5b to 6e29f19 Compare January 20, 2026 22:55
@andrew-anyscale andrew-anyscale force-pushed the andrew/revup/main/matrix-converter branch 2 times, most recently from 4c8d9d2 to 83f7a55 Compare January 26, 2026 19:22
@andrew-anyscale andrew-anyscale force-pushed the andrew/revup/main/matrix-testedd branch 2 times, most recently from ef5cae9 to 5e3f8d0 Compare January 26, 2026 19:32
@andrew-anyscale andrew-anyscale force-pushed the andrew/revup/main/matrix-converter branch from 83f7a55 to 2d792f3 Compare January 26, 2026 19:32
@andrew-anyscale andrew-anyscale force-pushed the andrew/revup/main/matrix-testedd branch from 5e3f8d0 to fa612a4 Compare January 26, 2026 19:42
@andrew-anyscale andrew-anyscale force-pushed the andrew/revup/main/matrix-converter branch from 2d792f3 to 4214f2b Compare January 26, 2026 19:42
@gitar-bot
Copy link

gitar-bot bot commented Jan 26, 2026

Code Review 👍 Approved with suggestions 0 resolved / 1 findings

Trivial DNM validation PR removing a redundant comment. The previous finding about inconsistent comment removal remains valid as only one test case comment was removed while others may still exist.

💡 Quality: Inconsistent comment removal breaks test case pattern

📄 wanda/cache_epoch_test.go:19

The removal of the // Saturday inline comment breaks consistency with other test cases in the file. Based on the feedback, other day-of-week test cases (like "sunday" shown at line 23) have similar inline comments documenting which day of the week the test date represents.

While the test name already indicates "saturday", the inline comments serve as quick visual verification that the date value (morningOf(2025, time.May, 31)) actually corresponds to a Saturday. This is helpful when reviewing test cases at a glance.

Suggestion: Either:

  1. Restore the // Saturday comment to maintain consistency, or
  2. Remove all day-of-week comments from the test file if they're truly redundant (not just this one)

Given this is a "DNM: validation" draft PR, this may be intentionally minimal, but for code quality purposes, consistency should be maintained.

Options

Auto-apply is off → Gitar will not commit updates to this branch.
Display: compact → Showing less information.

Comment with these commands to change:

Auto-apply Compact
gitar auto-apply:on         
gitar display:verbose         

Was this helpful? React with 👍 / 👎 | Gitar

@andrew-anyscale andrew-anyscale force-pushed the andrew/revup/main/matrix-converter branch from 4214f2b to 0be5ac9 Compare February 6, 2026 01:18
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.

1 participant