Skip to content

Conversation

Angeluz-07
Copy link
Contributor

fixes #399

@Angeluz-07 Angeluz-07 force-pushed the 399-set-sec-to-zero branch from 0b7c0cf to 7849d48 Compare June 23, 2020 19:47
Copy link
Contributor

@juanultimate juanultimate left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good job @Angeluz-07 . 💯
It would be better if you add some unit tests around these new feature

const endDateChanged = this.entry.end_date !== event.entry.end_date;

if (startDateChanged) {
const startDate = new Date(event.entry.start_date);
Copy link
Contributor

@juanultimate juanultimate Jun 23, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Lines 71, 72, 73 are the same than 89,90,91. Do not you think it would be great if we extract them to a method?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Your're right. I thought the same. I just didn't know the code style to include functions. I will ask for help to Rene for this and also to fix the tests. Thanks for pointing it out.

@Angeluz-07 Angeluz-07 force-pushed the 399-set-sec-to-zero branch from 0df5437 to 82a4ff9 Compare June 24, 2020 17:41
@codecov
Copy link

codecov bot commented Jun 24, 2020

Codecov Report

Merging #416 into master will decrease coverage by 0.30%.
The diff coverage is 64.28%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #416      +/-   ##
==========================================
- Coverage   90.66%   90.36%   -0.31%     
==========================================
  Files          72       72              
  Lines        1286     1297      +11     
  Branches       83       87       +4     
==========================================
+ Hits         1166     1172       +6     
- Misses         93       96       +3     
- Partials       27       29       +2     
Impacted Files Coverage Δ
...dules/time-entries/pages/time-entries.component.ts 83.01% <64.28%> (-7.46%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 086601b...4944d40. Read the comment docs.

@enriquezrene enriquezrene merged commit 2ec6cb9 into master Jun 24, 2020
@enriquezrene enriquezrene deleted the 399-set-sec-to-zero branch June 24, 2020 19:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

set seconds to 00
3 participants