Conversation
Summary of ChangesHello, 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 new Highlights
🧠 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. Changelog
Activity
Using Gemini Code AssistThe 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
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 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
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
| import { CargoFieldError, IsUppercase } from '../../src' | ||
| import { CargoClassMetadata } from '../../src/metadata' | ||
|
|
||
| describe('isUppercase decorator', () => { |
There was a problem hiding this comment.
테스트 코드의 가독성과 유지보수성을 높이기 위해, 여러 it 블록에서 반복되는 isUppercaseRule을 가져오는 로직을 describe 블록 상단으로 옮기는 것을 제안합니다. 이렇게 하면 중복 코드를 줄이고 각 테스트의 의도를 더 명확하게 파악할 수 있습니다.
예를 들어, 다음과 같이 리팩토링할 수 있습니다:
describe('isUppercase decorator', () => {
class Sample {
@IsUppercase()
uppercaseValue!: string
noValidatorValue!: string
}
const classMeta = new CargoClassMetadata(Sample.prototype);
const meta = classMeta.getFieldMetadata('uppercaseValue');
const isUppercaseRule = meta.getValidators()?.find(v => v.type === 'isUppercase');
it('should have isUppercase validator', () => {
expect(isUppercaseRule).toBeDefined();
expect(isUppercaseRule?.message).toBe('uppercaseValue should be uppercase');
});
it('should pass for all-uppercase strings', () => {
expect(isUppercaseRule?.validate('HELLO')).toBeNull();
// ...
});
// ... other tests using isUppercaseRule
});References
- 코드는 항상 읽기 쉽고 이해하기 쉬워야 합니다. 테스트 코드에서 반복되는 로직을 줄이면 가독성과 유지보수성이 향상됩니다. (link)
| new ValidatorRule( | ||
| propertyKey, | ||
| 'isUppercase', | ||
| (value: unknown) => typeof value === 'string' && value === value.toUpperCase(), |
There was a problem hiding this comment.
현재 구현은 value.toUpperCase()를 호출하여 새로운 문자열을 할당합니다. 이 방식은 매우 긴 문자열을 검증할 때 미미한 성능 저하를 유발할 수 있습니다.
Alpha, Alphanumeric와 같은 다른 검증기와의 일관성을 위해, 그리고 잠재적인 성능 향상을 위해 정규식을 사용하는 것을 제안합니다.
| (value: unknown) => typeof value === 'string' && value === value.toUpperCase(), | |
| (value: unknown) => typeof value === 'string' && !/[a-z]/.test(value), |
References
- 유사한 기능을 하는 검증기들은 일관된 방식으로 구현하는 것이 좋습니다. 또한, 불필요한 문자열 할당을 피해 성능을 개선할 수 있습니다.
…or better performance
|
/gemini review |
| new ValidatorRule( | ||
| propertyKey, | ||
| 'isUppercase', | ||
| (value: unknown) => typeof value === 'string' && !/[a-z]/.test(value), |
There was a problem hiding this comment.
@IsUppercase 유효성 검사기의 현재 구현은 ASCII 소문자(a-z)만 확인하므로, 다른 언어의 소문자나 악센트가 있는 문자를 올바르게 처리하지 못합니다. 예를 들어, 'ü'나 'привет'과 같은 문자열이 유효성 검사를 통과하게 됩니다.
String.prototype.toUpperCase()를 사용하여 모든 유니코드 문자를 올바르게 처리하는 것이 더 안정적인 방법입니다.
| (value: unknown) => typeof value === 'string' && !/[a-z]/.test(value), | |
| (value: unknown) => typeof value === 'string' && value.toUpperCase() === value, |
| import { CargoFieldError, IsUppercase } from '../../src' | ||
| import { CargoClassMetadata } from '../../src/metadata' | ||
|
|
||
| describe('isUppercase decorator', () => { |
There was a problem hiding this comment.
IsUppercase 유효성 검사기의 견고성을 높이기 위해 유니코드 문자에 대한 테스트 케이스를 추가하는 것을 권장합니다. 현재 테스트는 ASCII 문자만 다루고 있어, 다국어 환경에서의 동작을 보장하기 어렵습니다.
아래와 같은 테스트 케이스를 추가하여 악센트가 있는 문자나 다른 언어의 대소문자를 올바르게 처리하는지 확인할 수 있습니다.
it('should handle unicode characters correctly', () => {
// The following will pass with the suggested implementation change
expect(isUppercaseRule?.validate('ÄÖÜ')).toBeNull(); // Uppercase with accents
expect(isUppercaseRule?.validate('ÄöÜ')).toBeInstanceOf(CargoFieldError); // Mixed case with accents
expect(isUppercaseRule?.validate('ПРИВЕТ')).toBeNull(); // Uppercase Cyrillic
expect(isUppercaseRule?.validate('Привет')).toBeInstanceOf(CargoFieldError); // Mixed case Cyrillic
expect(isUppercaseRule?.validate('你好')).toBeNull(); // Non-cased characters should pass
});
yuchem2
left a comment
yuchem2
left a comment
There was a problem hiding this comment.
영어 문서가 누락된 것 같습니다! 영어 문서 작성해주세요!
|
감사합니다! 영어 문서 작성해서 올렸습니다! |
3 participants
• add test code for @IsUppercase decorator
• add example code for @IsUppercase decorator
• add documentation for @IsUppercase decorator