Skip to content

Conversation

@arjunlalb
Copy link
Contributor

Description

Add support for long string format in multi unit string

Testing

Please describe the tests that you ran to verify your changes. Please summarize what did you test and what needs to be tested e.g. deployed and tested helm chart locally.

Checklist:

  • My changes generate no new warnings
  • I have added tests that prove my fix is effective or that my feature works
  • Any dependent changes have been merged and published in downstream modules

@arjunlalb arjunlalb requested a review from a team as a code owner January 14, 2021 05:01
@codecov
Copy link

codecov bot commented Jan 14, 2021

Codecov Report

Merging #500 (b052bee) into main (9946ae9) will increase coverage by 0.00%.
The diff coverage is 91.66%.

Impacted file tree graph

@@           Coverage Diff           @@
##             main     #500   +/-   ##
=======================================
  Coverage   85.69%   85.69%           
=======================================
  Files         751      751           
  Lines       15409    15417    +8     
  Branches     1829     1834    +5     
=======================================
+ Hits        13205    13212    +7     
- Misses       2173     2174    +1     
  Partials       31       31           
Impacted Files Coverage Δ
projects/common/src/time/time-duration.ts 94.73% <91.66%> (-0.67%) ⬇️

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 9946ae9...b052bee. Read the comment docs.

@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

| TimeUnit.Second
| TimeUnit.Millisecond;

export enum UnitStringType {
Copy link
Contributor

Choose a reason for hiding this comment

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

I noticed this got switched back from const again? was const causing issues?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yep. using const fails when the enum is used as a param value in a method.

Copy link
Contributor

Choose a reason for hiding this comment

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

hmm - shouldn't be. I suspect that's a local caching artifact from the switch between const and non-const. Could you trying clearing your jest cache npm run test -- --clear-cache and then running with the const version?

@github-actions

This comment has been minimized.

@arjunlalb arjunlalb merged commit ae07526 into main Jan 16, 2021
@arjunlalb arjunlalb deleted the time-duration-update branch January 16, 2021 15:02
@github-actions
Copy link

Unit Test Results

    4 files  ±0  231 suites  ±0   14m 38s ⏱️ + 2m 44s
814 tests ±0  814 ✔️ ±0  0 💤 ±0  0 ❌ ±0 
818 runs  ±0  818 ✔️ ±0  0 💤 ±0  0 ❌ ±0 

Results for commit ae07526. ± Comparison against base commit 9946ae9.

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.

3 participants