This repository has been archived by the owner on Jul 12, 2022. It is now read-only.
Butler executions filters #922
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Signed-off-by: Leandro Rodrigues <leandro.rodrigues@zup.com.br>
Signed-off-by: Leandro Rodrigues <leandro.rodrigues@zup.com.br>
leandrorodrigueszup
requested review from
cpgo,
lucasbfernandes and
thallesfreitaszup
boring-cyborg
bot
added
the
butler
Codecov Report
@@ Coverage Diff @@
## main #922 +/- ##
=========================================
Coverage 77.50% 77.51%
Complexity 1978 1978
=========================================
Files 1192 1192
Lines 16402 16406 +4
Branches 1490 1490
=========================================
+ Hits 12713 12717 +4
Misses 3217 3217
Partials 472 472
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report at Codecov.
|
cpgo
previously approved these changes
Feb 23, 2021
lucasbfernandes
previously approved these changes
Feb 23, 2021
Signed-off-by: Leandro Rodrigues <leandro.rodrigues@zup.com.br>
leandrorodrigueszup
dismissed stale reviews from lucasbfernandes and cpgo
via
36d4178
Signed-off-by: Leandro Rodrigues <leandro.rodrigues@zup.com.br>
cpgo
reviewed
Mar 1, 2021
| @@ -35,10 +35,10 @@ export class ExecutionQuery { | |||
| public page: number | |||
|
|
|||
| @ApiProperty() | |||
| @IsBooleanString() | |||
There was a problem hiding this comment.
Why remove the IsBooleanString validation? Is it not needed?
There was a problem hiding this comment.
Because its not mandatory anymore!
lucasbfernandes
suggested changes
Mar 1, 2021
Comment on lines
30
to
38
| private getPageSize(actualPageSize: number): number { | ||
| return Math.min(actualPageSize, this.maxPageSize) | ||
| } | ||
|
|
||
| private isLast(page: number, totalPages: number): boolean { | ||
| return totalPages > 0 ? | ||
| page === (totalPages -1) : | ||
| true | ||
| } |
There was a problem hiding this comment.
Wouldn't be better to put this in a utils file? I think we might reuse this elsewhere
There was a problem hiding this comment.
I usually try to avoid util files. So, I changed the paginated response interface, so the isLast method could be in it.
Signed-off-by: Leandro Rodrigues <leandro.rodrigues@zup.com.br>
Signed-off-by: Leandro Rodrigues <leandro.rodrigues@zup.com.br>
Signed-off-by: Leandro Rodrigues <leandro.rodrigues@zup.com.br>
Signed-off-by: Leandro Rodrigues <leandro.rodrigues@zup.com.br>
Signed-off-by: Leandro Rodrigues <leandro.rodrigues@zup.com.br>
Signed-off-by: Leandro Rodrigues <leandro.rodrigues@zup.com.br>
lucasbfernandes
approved these changes
Mar 3, 2021
cpgo
approved these changes
Mar 4, 2021
thallesfreitaszup
approved these changes
Mar 10, 2021
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
No description provided.