-
Notifications
You must be signed in to change notification settings - Fork 1
TT-653 - TT-653-Time-Entries-Incorrect-Message #858
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
Conversation
|
Kudos, SonarCloud Quality Gate passed!
|
Codecov Report
@@ Coverage Diff @@
## master #858 +/- ##
==========================================
- Coverage 97.48% 97.44% -0.05%
==========================================
Files 107 109 +2
Lines 2430 2464 +34
Branches 203 205 +2
==========================================
+ Hits 2369 2401 +32
Misses 23 23
- Partials 38 40 +2
Continue to review full report at Codecov.
|
[infrastructure/][stage] Terraform Plan 📖
|
[infrastructure/][prod] Terraform Plan 📖
|
| if(date == null || date == undefined || date == '') return 'In progress'; | ||
| return moment.utc(date).utcOffset(-1*offset).format("HH:mm"); | ||
| parseDateTimeOffset(date: string, offset): string { | ||
| if (date === null || date === undefined || date === '') { |
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'm just curious, why not if (!date) 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.
I also noticed that offset doesn't have typing, I think it's a number offset: number
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.
Well that code was already here, I just changed the format that was complaining when I tried to do a commit.
The change related to this task is on this file
src/app/modules/shared/components/details-fields/details-fields.component.ts
But what you are suggesting seems logical to me, do you think we need another task to "clean up" the code ?
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.
these would be small refactors so you can fit them here if you want yeah, if you are going to do so just make sure that it's well tested to not break anything :P
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 just add here what the plugin used for checking the code was asking, which is adding blank spaces, or adding brackets in an "if", as you mentioned refactoring the code in this case is small, but could break something, and I feel comfortable doing them in another task IMO :p .








No description provided.