-
Notifications
You must be signed in to change notification settings - Fork 14
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
#1966 - Fix sonarcloud critical and major issues (part 2) #1973
Conversation
@@ -129,7 +129,7 @@ export class DisbursementScheduleSharedService extends RecordDataModelService<Di | |||
assessment.modifier = auditUser; | |||
// Adjust the saved grants disbursements with the values already disbursed. | |||
// !Intended to process only grants (CanadaGrant/BCGrant) | |||
this.applyGrantsAlreadyDisbursedValues( | |||
await this.applyGrantsAlreadyDisbursedValues( |
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.
I believe that the method is wrongly marked with async
. Please check if the async
can be removed from the method and remove the await
add here.
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.
Thanks for checking the issues, please take a look at the comments.
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.
Good job. Just take a look at Andrew's comments.
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.
Done with the review, please have a look on the other Devs comments.
@@ -487,10 +487,10 @@ export class DisbursementScheduleSharedService extends RecordDataModelService<Di | |||
* @param totalAlreadyDisbursedValues sum of the awards already | |||
* disbursed per award code. | |||
*/ | |||
private async applyGrantsAlreadyDisbursedValues( | |||
private applyGrantsAlreadyDisbursedValues( |
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.
👍
Kudos, SonarCloud Quality Gate passed!
|
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.
Thanks for doing the changes. Looks good 👍
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.
👍 Good work @sh16011993
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.
👍
As a part of this PR, fixed the major issues (bugs) reported by SonarCloud.