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

feat(isISO8601): Add new option to isISO8601 that checks if date and time are strictly separated by a T #1486

Merged
merged 4 commits into from
Nov 24, 2020

Conversation

joestone51
Copy link
Contributor

@joestone51 joestone51 commented Oct 13, 2020

As per the Wikipedia entry for ISO8601, the T that separates date and time is required. However, separating date and time by another character (such as whitespace, \s, in the current regular expression) is allowed by RFC 3339.

In order to not break any existing dependent code, I have added a new option to isISO8601 that allows the user to pass strictSeparator = true in the options. This would validate the date time string to always have the date and time separated by the literal T.

Checklist

  • PR contains only changes related; no stray files, etc.
  • README updated (where applicable)
  • Tests written (where applicable)

@joestone51 joestone51 changed the title add strictSeparator option to isISO8601 feat(isISO8601): Add new option to isISO8601 that checks if date and time are strictly separated by a T Oct 13, 2020
@codecov
Copy link

codecov bot commented Oct 13, 2020

Codecov Report

Merging #1486 (7d080b6) into master (0b34b93) will increase coverage by 0.00%.
The diff coverage is 100.00%.

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #1486   +/-   ##
=======================================
  Coverage   99.92%   99.92%           
=======================================
  Files          96       96           
  Lines        1286     1287    +1     
=======================================
+ Hits         1285     1286    +1     
  Misses          1        1           
Impacted Files Coverage Δ
src/lib/isISO8601.js 100.00% <100.00%> (ø)

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 0b34b93...d1e68fc. Read the comment docs.

@joestone51
Copy link
Contributor Author

Any chance of this seeing a review anytime soon?

@joestone51
Copy link
Contributor Author

It has been 22 days since I submitted this Pr -- have i done something incorrectly? I've seen several other PRs get reviewed that were submitted after this one.

@profnandaa
Copy link
Member

@brostone51 -- sorry for the delay on reviewing this, on it now.

Copy link
Member

@profnandaa profnandaa left a comment

Choose a reason for hiding this comment

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

LGTM, thanks for your contribution! 🎉

@profnandaa
Copy link
Member

/cc. @ezkemboi @tux-tn -- please have a look, would like to land this, this weekend.

Copy link
Member

@tux-tn tux-tn left a comment

Choose a reason for hiding this comment

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

LGTM ! Great work !

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.

3 participants