fixes #259 allowing trailing Z for all variants of time parts #265

mcauer · 2020-09-07T14:31:50Z

Changes proposed in this pull request:

Type of change

Please delete if not relevant:

Bug fix (non-breaking change which fixes an issue)
New feature (non-breaking change which adds functionality)
Breaking change (fix or feature that would cause existing functionality to not work as expected)
This change requires a documentation update

Description

Extending the ISODateTimeParser to allow a trailing Z for all variants of time parts in a DateTime String. e.g. 2020-05-01T15Z

Corresponding issue

Closes #259

New or changed dependencies:

Checklist

My code follows the code-style rules and I have checked on the static analyses and benchmark (if applicable) results
I have commented my code
I have written javadoc (required for public methods)
I have added sufficient unit tests
I have made corresponding changes to the documentation
I have updated the CHANGELOG.md
I have adjusted the examples and benchmark if necessary

tyrasd

found some mostly minor issues here, and would be glad to hear what you think about our level of ISO standard compliance. 😊

oshdb-util/src/main/java/org/heigit/bigspatialdata/oshdb/util/time/ISODateTimeParser.java

tyrasd · 2020-09-08T07:59:58Z

ah, @mcauer – it seems like you did not yet add the email michael.auer@heigit.org to your github account, which you used to sign off the commits (66a1926 and aa4f780). Because of this the commits are not detected to belong to you.

Also, it would be superb if you could perhaps set up signing of your git commits using gpg: https://docs.github.com/en/github/authenticating-to-github/signing-commits 😃

mcauer · 2020-09-09T09:43:51Z

ah, @mcauer – it seems like you did not yet add the email michael.auer@heigit.org to your github account, which you used to sign off the commits (66a1926 and aa4f780). Because of this the commits are not detected to belong to you.

Also, it would be superb if you could perhaps set up signing of your git commits using gpg: https://docs.github.com/en/github/authenticating-to-github/signing-commits smiley

@tyrasd thanks for pointing me on it. Added my email and created a gpg key for future commits.

tyrasd · 2020-09-09T13:10:20Z

created a gpg key for future commits

looks like it is not yet working 100% correctly on your side, since 4eb0dc6 seems to be unsigned, AFAICs. 😉

mcauer · 2020-09-09T13:18:55Z

created a gpg key for future commits

looks like it is not yet working 100% correctly on your side, since 4eb0dc6 seems to be unsigned, AFAICs. wink

I used IntelliJ UI to commit. Can this be configured in IntelliJ?

mcauer · 2020-09-09T13:31:31Z

created a gpg key for future commits

looks like it is not yet working 100% correctly on your side, since 4eb0dc6 seems to be unsigned, AFAICs. wink

I used IntelliJ UI to commit. Can this be configured in IntelliJ?

Ok now it worked. puh. :)

tyrasd · 2020-09-09T13:47:59Z

👍

a few unit tests for the class would be nice, if you don't mind…

mcauer · 2020-09-09T16:17:15Z

+1

a few unit tests for the class would be nice, if you don't mind…

ok, still working on that.

oshdb-util/src/main/java/org/heigit/bigspatialdata/oshdb/util/time/ISODateTimeParser.java

joker234

I think some tests with valid ISO 8601 timestrings but with a not supported timezone (non-UTC) are missing. And the Changelog is missing!

oshdb-util/src/test/java/org/heigit/bigspatialdata/oshdb/util/time/IsoDateTimeParserTest.java

oshdb-util/src/main/java/org/heigit/bigspatialdata/oshdb/util/time/IsoDateTimeParser.java

tyrasd · 2020-11-03T17:13:46Z

I think some tests with valid ISO 8601 timestrings but with a not supported timezone (non-UTC) are missing.

@joker234 isn't that already there with tests like throwsPosTimezoneHHParseIsoDateTime (etc.) here? Or do you mean something else?

tyrasd · 2020-11-03T18:09:17Z

And the Changelog is missing!

done

joker234 · 2020-11-03T18:25:12Z

I think some tests with valid ISO 8601 timestrings but with a not supported timezone (non-UTC) are missing.

@joker234 isn't that already there with tests like throwsPosTimezoneHHParseIsoDateTime (etc.) here? Or do you mean something else?

Nevermind, that's alright.

…eTime Strings

…ignators than Z

* rename class "ISO" to "Iso" * fix too long lines * improve javadocs * some light refactoring

* throw errors in constructor * fix code style and javadoc issues * refactor to (hopefully) simpify things

Co-authored-by: Johannes Visintini <johannes.visintini@heigit.org>

mcauer requested a review from tyrasd September 7, 2020 14:38

tyrasd added this to the release 0.6.0 milestone Sep 7, 2020

tyrasd reviewed Sep 7, 2020

View reviewed changes

tyrasd mentioned this pull request Sep 9, 2020

document timezone of time parameter GIScience/ohsome-api#48

Closed

tyrasd reviewed Sep 10, 2020

View reviewed changes

oshdb-util/src/main/java/org/heigit/bigspatialdata/oshdb/util/time/ISODateTimeParser.java Outdated Show resolved Hide resolved

tyrasd reviewed Sep 10, 2020

View reviewed changes

oshdb-util/src/main/java/org/heigit/bigspatialdata/oshdb/util/time/ISODateTimeParser.java Outdated Show resolved Hide resolved

tyrasd self-assigned this Sep 11, 2020

tyrasd force-pushed the fix-issue-259-timestamp-formats branch 2 times, most recently from f3789c5 to f8148b1 Compare September 14, 2020 12:54

tyrasd added waiting for review This pull request needs a code review code quality Related to our standards for 'good' code user experience Enhances the usability of OSHDB labels Sep 16, 2020

tyrasd force-pushed the fix-issue-259-timestamp-formats branch from f8148b1 to 77c0085 Compare September 16, 2020 08:44

tyrasd requested a review from rtroilo September 16, 2020 08:44

tyrasd requested a review from joker234 October 26, 2020 17:59

tyrasd added the breaking This will introduce an incombability to previous versions. Documentation update compulsory label Oct 26, 2020

joker234 mentioned this pull request Oct 30, 2020

fix checkstyle warnings in oshdb-util #275

Merged

11 tasks

joker234 force-pushed the fix-issue-259-timestamp-formats branch from a49b2e1 to 87b5338 Compare November 3, 2020 15:40

joker234 requested changes Nov 3, 2020

View reviewed changes

tyrasd added a commit that referenced this pull request Nov 3, 2020

append changelog with breaking change in #265

765be03

joker234 approved these changes Nov 3, 2020

View reviewed changes

mcauer and others added 15 commits November 3, 2020 19:36

fixes #259 allowing trailing Z for all variants of time parts

bd43c7f

fixed some code format issues

a96c3ab

fixed some further code format issues

e79c69b

improved JavaDoc for more clarity of input format of supported ISODat…

e838c3f

…eTime Strings

fix another line length issue

99411dd

clarify that only UTC timestamps are allowed

71e7918

add further assertions to avoid negative dates and other timezone des…

1b9416f

…ignators than Z

add Unit Tests for ISODateTimeParser methods

aaea5d5

fix code style issues with iso date time parser

8fa6cf1

* rename class "ISO" to "Iso" * fix too long lines * improve javadocs * some light refactoring

refactor oshdb timestamps class

3133c09

* throw errors in constructor * fix code style and javadoc issues * refactor to (hopefully) simpify things

fix javadoc paragraphs

643cdb9

fix order of imports and more checkstyle warnings

fea0743

rename test methods to match class signature & checkstyle rules

16d979b

Co-authored-by: Johannes Visintini <johannes.visintini@heigit.org>

test for proper exception classes in time parser tests

a528244

append changelog with breaking change in #265

682b2ff

joker234 force-pushed the fix-issue-259-timestamp-formats branch from 765be03 to 682b2ff Compare November 3, 2020 18:37

joker234 merged commit 232beea into master Nov 3, 2020

joker234 deleted the fix-issue-259-timestamp-formats branch November 3, 2020 19:00

tyrasd removed the waiting for review This pull request needs a code review label Dec 3, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fixes #259 allowing trailing Z for all variants of time parts #265

fixes #259 allowing trailing Z for all variants of time parts #265

mcauer commented Sep 7, 2020 •

edited by tyrasd

Loading

tyrasd left a comment

tyrasd commented Sep 8, 2020

mcauer commented Sep 9, 2020

tyrasd commented Sep 9, 2020 •

edited

Loading

mcauer commented Sep 9, 2020

mcauer commented Sep 9, 2020

tyrasd commented Sep 9, 2020

mcauer commented Sep 9, 2020

joker234 left a comment •

edited

Loading

tyrasd commented Nov 3, 2020

tyrasd commented Nov 3, 2020

joker234 commented Nov 3, 2020

fixes #259 allowing trailing Z for all variants of time parts #265

fixes #259 allowing trailing Z for all variants of time parts #265

Conversation

mcauer commented Sep 7, 2020 • edited by tyrasd Loading

Changes proposed in this pull request:

Type of change

Description

Corresponding issue

New or changed dependencies:

Checklist

tyrasd left a comment

Choose a reason for hiding this comment

tyrasd commented Sep 8, 2020

mcauer commented Sep 9, 2020

tyrasd commented Sep 9, 2020 • edited Loading

mcauer commented Sep 9, 2020

mcauer commented Sep 9, 2020

tyrasd commented Sep 9, 2020

mcauer commented Sep 9, 2020

joker234 left a comment • edited Loading

Choose a reason for hiding this comment

tyrasd commented Nov 3, 2020

tyrasd commented Nov 3, 2020

joker234 commented Nov 3, 2020

mcauer commented Sep 7, 2020 •

edited by tyrasd

Loading

tyrasd commented Sep 9, 2020 •

edited

Loading

joker234 left a comment •

edited

Loading