fix#68
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 @HoseokWon99, 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 addresses a minor fix related to how range parameters are handled across the application. It standardizes the input format for range values, such as age and experience, by changing the expected delimiter from a hyphen to a comma. This ensures consistency in API parameter parsing and validation, improving the robustness of range-based queries.
Highlights
- API Parameter Pattern Update: Modified the
ApiPropertypattern forageinSearchAdjacentUsersQueryto accept comma-separated numerical ranges instead of hyphen-separated ones. - DTO Filter Pattern Update: Updated the
ApiPropertypattern forexperienceinSearchUsersFiltersto also expect comma-separated numerical ranges. - Range Decorator Logic Adjustment: Adjusted the
IsRangedecorator to match and parse numerical ranges using a comma as the delimiter, aligning it with the updated API and DTO patterns.
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 updates the format for range parameters from min-max to min,max and allows for multi-digit numbers. The changes are consistent across the API query DTOs and the custom IsRange validator. My review identifies an inconsistency between the validation regex and the transformation logic within the IsRange decorator. I've suggested improvements to make the validation more robust by allowing optional whitespace, and to update the corresponding API documentation patterns for consistency.
| @IsRange() | ||
| @IsOptional() | ||
| @ApiProperty({ name: "age", type: "string", pattern: "^(\\d)-(\\d)$", required: false }) | ||
| @ApiProperty({ name: "age", type: "string", pattern: "^(\\d+),(\\d+)$", required: false }) |
There was a problem hiding this comment.
To maintain consistency with the suggested change in the IsRange decorator's validation regex, this ApiProperty pattern should also be updated to allow for optional whitespace around the comma. This ensures the API documentation accurately reflects the validation logic.
| @ApiProperty({ name: "age", type: "string", pattern: "^(\\d+),(\\d+)$", required: false }) | |
| @ApiProperty({ name: "age", type: "string", pattern: "^(\\d+)\\s*,\\s*(\\d+)$", required: false }) |
| @IsRange() | ||
| @IsOptional() | ||
| @ApiProperty({ type: "string", pattern: "^(\\d)-(\\d)$", required: false }) | ||
| @ApiProperty({ type: "string", pattern: "^(\\d+),(\\d+)$", required: false }) |
There was a problem hiding this comment.
To maintain consistency with the suggested change in the IsRange decorator's validation regex, this ApiProperty pattern should also be updated to allow for optional whitespace around the comma. This ensures the API documentation accurately reflects the validation logic.
| @ApiProperty({ type: "string", pattern: "^(\\d+),(\\d+)$", required: false }) | |
| @ApiProperty({ type: "string", pattern: "^(\\d+)\\s*,\\s*(\\d+)$", required: false }) |
| export function IsRange (options?: ValidationOptions) { | ||
| return applyDecorators( | ||
| Matches(/^(\d+)-(\d+)$/), | ||
| Matches(/^(\d+),(\d+)$/), |
There was a problem hiding this comment.
The s.trim() call on line 26 suggests that whitespace around the comma is expected, but the current regex ^(\d+),(\d+)$ doesn't allow it. This makes the .trim() call redundant. To make the validation more robust and align with the transformation logic, consider allowing optional whitespace in the regex.
| Matches(/^(\d+),(\d+)$/), | |
| Matches(/^(\d+)\s*,\s*(\d+)$/), |
1 participant
No description provided.