Skip to content
This repository has been archived by the owner on Jan 16, 2022. It is now read-only.

fix: formatDate #308

Merged
merged 2 commits into from
Dec 6, 2019
Merged

fix: formatDate #308

merged 2 commits into from
Dec 6, 2019

Conversation

tmkn
Copy link
Contributor

@tmkn tmkn commented Nov 24, 2019

Description:
On Windows the tests failed for formatDate. It somehow took the timezone offset into account.
Adding the timezone offset before passing it to date-fns/format fixes it for Windows while remaining intact for Mac.

Since the tests now fully work on Windows we can add them to the CI pipeline?

@codecov
Copy link

codecov bot commented Nov 24, 2019

Codecov Report

Merging #308 into master will not change coverage.
The diff coverage is 100%.

@@           Coverage Diff           @@
##           master     #308   +/-   ##
=======================================
  Coverage   87.86%   87.86%           
=======================================
  Files         142      142           
  Lines         972      972           
  Branches      189      172   -17     
=======================================
  Hits          854      854           
- Misses        101      108    +7     
+ Partials       17       10    -7
Impacted Files Coverage Δ
src/utils/package.ts 95.83% <100%> (ø) ⬆️
src/components/Icon/styles.ts 88.88% <0%> (ø) ⬆️
src/utils/styles/media.ts 18.18% <0%> (ø) ⬆️
src/components/DetailSidebar/DetailSidebar.tsx 85% <0%> (ø) ⬆️

@tmkn
Copy link
Contributor Author

tmkn commented Nov 24, 2019

Also:

export function formatRepository(repository: any): string | null {

is only used in unit tests. There are 5 tests for it but it's not used anywhere else.
If it doesn't serve a function it can be removed.

@juanpicado
Copy link
Member

@tmkn good catch, Mmmmm .... if formatDate I would like to know when was removed ... if is really not need it, we should remove it

@tmkn
Copy link
Contributor Author

tmkn commented Nov 24, 2019

@juanpicado formatDate is still used, formatRepository is only used in unit tests.

@juanpicado juanpicado merged commit 33f873a into verdaccio:master Dec 6, 2019
@juanpicado
Copy link
Member

sorry the delay @tmkn I think I screw this PR :(

@tmkn
Copy link
Contributor Author

tmkn commented Dec 6, 2019

@juanpicado no worry, should I remove formatRepository in the next PR? Did you take a look?

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants