DateTime Format: remove testing before ICU77 / CLDR 47 #504
Conversation
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Summary of Changes
Hello @sven-oly, 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!
I've updated the run_config.json file to remove the datetime_fmt test type from various execution configurations. This change aligns with the goal of removing testing for older ICU/CLDR versions, specifically before ICU77 / CLDR 47, as indicated by the PR title.
Highlights
- Configuration Update: Removed the "datetime_fmt" entry from the "test_type" arrays across all relevant configurations within run_config.json. This streamlines the test suite by no longer running tests for datetime_fmt on older ICU/CLDR versions.
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 in your pull request via creating an issue comment (i.e. comment on the pull request page) using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands.
| 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 issue 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 or fill out our survey to provide feedback.
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
-
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. ↩
There was a problem hiding this comment.
Code Review
This pull request correctly removes the datetime_fmt test type from configurations for ICU versions older than 77, which aligns with the PR's goal. The changes are consistent and the JSON configuration remains valid. I have one suggestion to improve consistency in the test type ordering for one of the cpp configurations.
| @@ -124,7 +120,6 @@ | |||
| "exec": "cpp", | |||
| "test_type": [ | |||
| "collation", | |||
| "datetime_fmt", | |||
There was a problem hiding this comment.
While you're removing this test type, I noticed a small inconsistency in this test block compared to other cpp configurations.
For better consistency and maintainability, could you also reorder list_fmt and likely_subtags on lines 129-130? The order in other cpp blocks is likely_subtags then list_fmt.
Suggested change for lines 124-125 in the new version of the file:
"likely_subtags",
"list_fmt",
|
Replaced by #511 |
If we want to... #503