Skip to content
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

Date and time #26

Merged
merged 13 commits into from
May 20, 2020
Merged

Date and time #26

merged 13 commits into from
May 20, 2020

Conversation

ilariaventurini
Copy link
Contributor

@ilariaventurini ilariaventurini commented May 11, 2020

Please check the type of change your PR introduces:

  • Bugfix
  • Feature
  • Refactoring / Code style update (no functional changes, no api changes)
  • Build related changes
  • Documentation
  • Other (please describe):

Related to this issue.
Data juggler supports now also time, not only dates.

ref
Formats we support:

  • YYYY-MM-DD,

  • YYYY-MM-D,

  • YYYY-M-DD,

  • YYYY-M-D,

  • YYYY-MM-DD HH:mm,

  • YYYY-MM-DD HH:mm[Z],

  • YYYY-MM-DD[T]HH:mm,

  • YYYY-MM-DD[T]HH:mm[Z],

  • YYYY-MM-DD HH:mm:ss,

  • YYYY-MM-DD HH:mm:ss[Z],

  • YYYY-MM-DD[T]HH:mm:ss,

  • YYYY-MM-DD[T]HH:mm:ss[Z],

  • YYYY-MM-DD HH:mm:ss.SSS,

  • YYYY-MM-DD HH:mm:ss.SSS[Z],

  • YYYY-MM-DD[T]HH:mm:ss.SSS,

  • YYYY-MM-DD[T]HH:mm:ss.SSS[Z], // ISO8601

  • YYYY-MM-DD HH:mm:ss A,

  • YYYY-MM-DD HH:mm:ss a,

In the future we would like to support also these two formats that for some strange reasons they actually don`t work:

  • YYYY-MM-DD HH:mm:ssZ, ie. 2013-02-08 09:00:00+07:00, 2013-02-08 09:00:00+07:30
  • YYYY-MM-DD HH:mm:ssZZ, ie 2013-02-08 09:00:00-0700, 2013-02-08 09:00:00-0730

Where:

Format Example Description
YYYY 2018 Four-digit year
MM 01-12 Month, 2-digits
M 1-12 Month, beginning at 1
DD 01-31 Day of month, 2-digits
D 1-31 Day of month
HH 00-23 Hours, 2-digits
mm 00-59 Minutes, 2-digits
ss 00-59 Seconds, 2-digits
SSS 000-999 Milliseconds, 3-digits
A AM, PM Post or ante meridiem, upper-case
a am, pm Post or ante meridiem, lower-case

Examples:

1900-10-23
1942-11-1
2002-5-15
2000-01-10
2020-05-01 09:35
2020-05-01 09:35Z
2020-05-01T09:35
2020-05-01T09:35Z
2020-05-01 09:35:20
2019-01-15 13:12:29
2020-05-01 09:35:20Z
2020-05-01T09:35:20
2020-05-01T09:35:20Z
2020-05-01 09:35:20.000
2020-05-01 09:35:20.000Z
2020-05-01T09:35:20.000
2020-05-01T09:35:20.000Z
2020-05-13T08:24:45.701Z
2016-01-01 11:31:23 AM
2016-01-01 11:31:23 am
2016-01-01 23:31:23 pm

cat-1
2017-02-30
2020-04-31
2018-02-29
2008-09-31
17-02-2019
0000-01-01
0100-10-23
2009-23-5
1942-11-0
1942-00-25
2000-10-00
2019-01-15 24:00:00
2019-01-15 23:60:00
2019-01-15 23:59:60
2019-01-15 13:12:29.0
2020-05-01 01:01:01.12
2016-01-01 11:31:23 PM
2020-05-01 00
2020-02-31
2016/01/01
2020-05-01 01:60:00
2020-05-01 60
2020-05-01
Wed May 13 2020 10:25:23 GMT+0200 (Central European Summer Time)

I had to add watch-tsc command becausewatch command doesn't work because of this issue.
Probably we need a refactor, there are a lot of scripts.

Does this introduce a breaking change?

  • Yes
  • No

Copy link
Contributor

@caesarsol caesarsol left a comment

Choose a reason for hiding this comment

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

Cool! 💯

  1. I like disabling the comma, not because of the CSV but because it's non-standard and I have never seen it :)
  2. I would also add the ISO Format, which is simply the output of JS date.toISOString. Basically it's the T separator, a 4-digit milliseconds number after a decimal point ., and then some info on the timezone.
  3. I think, to make it readable and clearer, you can split the regexp in the possible formats:
  • YYYY-MM-DD

  • YYYY-M-D

  • the two above with [ T]HH:mm:ss (you can use boolean logic with matches)

  • the above with also milliseconds and/or timezone, but make it simple!

    You can also have a look at the existant NPM modules, maybe there's something ready for the purpose.
    The aim: obviously remove the comments! 😄

src/lib/dataInference.ts Outdated Show resolved Hide resolved
tslint.json Show resolved Hide resolved
tslint.json Show resolved Hide resolved
src/lib/dataInference.ts Outdated Show resolved Hide resolved
src/lib/dataInference.ts Outdated Show resolved Hide resolved
src/lib/dataInference.ts Outdated Show resolved Hide resolved
@caesarsol
Copy link
Contributor

I'd say you can merge this yourself after the two small changes I requested! 💯

src/lib/dataInference.ts Outdated Show resolved Hide resolved
src/lib/dataInference.ts Outdated Show resolved Hide resolved
Copy link
Contributor

@caesarsol caesarsol left a comment

Choose a reason for hiding this comment

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

Sounds good! Added couple final comments.

@caesarsol caesarsol merged commit 58a6a1b into master May 20, 2020
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