-
Notifications
You must be signed in to change notification settings - Fork 0
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
refactor: change range enum #56
Conversation
WalkthroughThis update involves modifications across various parts of the codebase, primarily focusing on refining enums, updating data types in DTOs, and schema validations. Notable changes include the removal of specific enum entries in Changes
Tip AI model upgrade
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 5
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (10)
- src/core/enums/db-time-interval.enum.ts (1 hunks)
- src/core/enums/range.enum.ts (1 hunks)
- src/core/schemas/range.schema.ts (1 hunks)
- src/modules/governance/dto/gov-overview.dto.ts (1 hunks)
- src/modules/governance/services/governance.service.ts (2 hunks)
- src/modules/supply/services/supply.service.ts (2 hunks)
- src/modules/supply/supply.controller.ts (3 hunks)
- src/modules/token/dtos/token-info.dto.ts (1 hunks)
- src/modules/token/services/token.service.ts (2 hunks)
- src/modules/token/token.controller.ts (2 hunks)
Files skipped from review due to trivial changes (2)
- src/core/enums/db-time-interval.enum.ts
- src/modules/governance/dto/gov-overview.dto.ts
Additional context used
Biome
src/modules/token/token.controller.ts
[error] 23-23: Decorators are not valid here.
Decorators are only valid on class declarations, class expressions, and class methods.
You can enable parameter decorators by setting theunsafeParameterDecoratorsEnabled
option totrue
in your configuration file.(parse)
src/modules/supply/supply.controller.ts
[error] 20-20: Decorators are not valid here.
Decorators are only valid on class declarations, class expressions, and class methods.
You can enable parameter decorators by setting theunsafeParameterDecoratorsEnabled
option totrue
in your configuration file.(parse)
[error] 33-33: Decorators are not valid here.
Decorators are only valid on class declarations, class expressions, and class methods.
You can enable parameter decorators by setting theunsafeParameterDecoratorsEnabled
option totrue
in your configuration file.(parse)
[error] 41-41: Decorators are not valid here.
Decorators are only valid on class declarations, class expressions, and class methods.
You can enable parameter decorators by setting theunsafeParameterDecoratorsEnabled
option totrue
in your configuration file.(parse)
Additional comments not posted (5)
src/core/enums/range.enum.ts (1)
1-7
: LGTM! The enum simplification is clear and consistent.The removal of
FIVE_MIN
,HOUR
, andTHREE_MONTH
values simplifies theRange
enum without compromising its functionality.src/core/schemas/range.schema.ts (1)
1-6
: LGTM! The schema correctly validates theRange
enum values.The introduction of
RangeSchema
using Joi ensures valid range values.src/modules/token/dtos/token-info.dto.ts (1)
5-5
: LGTM! The change aligns with the expected data type forapr
.Changing
apr
fromstring
tonumber
inTokenInfoDto
is logical and improves type consistency.src/modules/supply/services/supply.service.ts (2)
32-32
: Verify the impact of removingRange.HOUR
.Ensure that the removal of
Range.HOUR
fromrangeTimeIntervalMap
does not affect other parts of the codebase.Verification successful
No impact from removing
Range.HOUR
.The removal of
Range.HOUR
fromrangeTimeIntervalMap
does not affect other parts of the codebase as it is not used elsewhere.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the impact of removing `Range.HOUR`. # Test: Search for the usage of `Range.HOUR`. Expect: No breaking changes. rg --type typescript $'Range.HOUR'Length of output: 68
Script:
#!/bin/bash # Description: Verify the impact of removing `Range.HOUR`. # Test: Search for the usage of `Range.HOUR` in all files. rg 'Range.HOUR'Length of output: 15
187-187
: Verify the correctness of the new calculation logic forRange.YEAR
.Ensure that the new calculation logic in the
calculatePastDateByRange
method is correct and does not introduce any errors.Verification successful
The new calculation logic for
Range.YEAR
in thecalculatePastDateByRange
method is correct and does not introduce any errors.
- The logic correctly sets the date to one year earlier using
date.setFullYear(date.getFullYear() - 1)
.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the correctness of the new calculation logic for `Range.YEAR`. # Test: Check the usage of the `calculatePastDateByRange` method. Expect: Correct logic. rg --type typescript $'calculatePastDateByRange'Length of output: 82
Script:
#!/bin/bash # Description: Verify the correctness of the new calculation logic for `Range.YEAR`. # Test: Check the usage of the `calculatePastDateByRange` method. Expect: Correct logic. rg --glob '*.ts' 'calculatePastDateByRange'Length of output: 359
Script:
#!/bin/bash # Description: Extract the implementation and usage context of the `calculatePastDateByRange` method. # Extract the `calculatePastDateByRange` method implementation. ast-grep --lang typescript --pattern $'private calculatePastDateByRange(range: Range): Date { $$$ }' --json # Extract the surrounding context where `calculatePastDateByRange` is called. rg --context 5 'calculatePastDateByRange' src/modules/supply/services/supply.service.tsLength of output: 1633
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (1)
- src/modules/token/services/token.service.ts (2 hunks)
Additional context used
Biome
src/modules/token/services/token.service.ts
[error] 111-111: isNaN is unsafe. It attempts a type coercion. Use Number.isNaN instead.
See the MDN documentation for more details.
Unsafe fix: Use Number.isNaN instead.(lint/suspicious/noGlobalIsNan)
@@ -109,7 +108,7 @@ export class TokenService implements OnModuleInit { | |||
change: mcap!.change, | |||
}, | |||
volume: res.volume_24h, | |||
apr, | |||
apr: isNaN(Number(apr)) ? 0 : Number(apr), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Use Number.isNaN
instead of isNaN
isNaN
is unsafe as it attempts a type coercion. Use Number.isNaN
to ensure type safety.
- apr: isNaN(Number(apr)) ? 0 : Number(apr),
+ apr: Number.isNaN(Number(apr)) ? 0 : Number(apr),
Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
apr: isNaN(Number(apr)) ? 0 : Number(apr), | |
apr: Number.isNaN(Number(apr)) ? 0 : Number(apr), |
Tools
Biome
[error] 111-111: isNaN is unsafe. It attempts a type coercion. Use Number.isNaN instead.
See the MDN documentation for more details.
Unsafe fix: Use Number.isNaN instead.(lint/suspicious/noGlobalIsNan)
No description provided.