Skip to content

Conversation

OskarDamkjaer
Copy link
Contributor

Changes:

  • Adds luxon as a date/duration management lib
  • Formats durations better for both exports and cyper result frame

Before:
CleanShot 2021-09-13 at 17 19 28

After:
CleanShot 2021-09-13 at 17 19 46

Preview @ https://format_duration.surge.sh

@@ -30,7 +30,8 @@
"e2e-local": "CYPRESS_E2E_TEST_ENV=\"local\" cypress run",
"e2e-local-open": "CYPRESS_E2E_TEST_ENV=\"local\" cypress open",
"format": "prettier-eslint --write 'src/**/!(*.min).{js,jsx,ts,tsx,css,json}' 'e2e_tests/**/*.{js,jsx,ts,tsx,css,json}' 'scripts/**/*.{js,jsx,ts,tsx,css,json}' 'build_scripts/**/*.{js,jsx,ts,tsx,css,json}'",
"jest": "jest --coverage",
Copy link
Contributor

Choose a reason for hiding this comment

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

This will affect CI, so we should either just add a new jest-no-cov script and leave the existing jest as it was, or else we should check what the change will mean/do on CI.

@@ -443,9 +444,13 @@ export function mapNeo4jValuesToPlainValues(values: any): any {
*/
function neo4jValueToPlainValue(value: any) {
switch (get(value, 'constructor')) {
case neo4j.types.Duration:
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we move this code that converts the value to a duration string to a shared util function or something?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done!

Copy link
Contributor

@jharris4 jharris4 left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Contributor

@jharris4 jharris4 left a comment

Choose a reason for hiding this comment

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

Approving, although hopefully removing the coverage from CI won't bite us later...

@OskarDamkjaer OskarDamkjaer merged commit ac8b22c into neo4j:master Sep 22, 2021
@OskarDamkjaer OskarDamkjaer changed the title Add luxon to fix duration formatting issue Properly normalize duration format Oct 12, 2021
@OskarDamkjaer OskarDamkjaer changed the title Properly normalize duration format Properly normalize duration values Oct 12, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants