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.

Sign up for GitHub

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

Jump to bottom

Support parsing timestamps without seconds #1553

Merged

Baccata merged 8 commits into series/0.18 from support-parsing-timestamps-without-seconds

Jun 3, 2024

Contributor

dhpiggott commented May 30, 2024 •

edited

Loading

PR Checklist (not all items are relevant to all PRs)

Added unit-tests (for runtime code)
Added bootstrapped code + smoke tests (when the rendering logic is modified)
Added build-plugins integration tests (when reflection loading is required at codegen-time)
Added alloy compliance tests (when simpleRestJson protocol behaviour is expanded/updated)
Updated dynamic module to match generated-code behaviour
Added documentation
Updated changelog

dhpiggott added 4 commits

May 30, 2024 16:38


          Add property tests replicating the issue

b4d5bc4


          Fix Timestamp.parseDateTime impls

77839ca


          Update changelog

bebbbe7


          Remove invalid prefix from dateTimeWithoutSecondsFormatter name

c3a20d1

dhpiggott commented

View reviewed changes

modules/core/src-js/smithy4s/Timestamp.scala

		@@ -290,70 +290,78 @@ object Timestamp {

Contributor Author

dhpiggott May 30, 2024

These changes are the same as the JVM changes. See the notes on it.

dhpiggott commented

View reviewed changes

modules/core/src-jvm-native/smithy4s/Timestamp.scala Outdated Show resolved Hide resolved

dhpiggott commented

View reviewed changes

modules/core/src-jvm-native/smithy4s/Timestamp.scala Outdated Show resolved Hide resolved

dhpiggott commented

View reviewed changes

modules/core/src-jvm-native/smithy4s/Timestamp.scala Outdated Show resolved Hide resolved

dhpiggott commented

View reviewed changes

modules/core/src-jvm-native/smithy4s/Timestamp.scala Outdated Show resolved Hide resolved


          Fix minute and second limits (s/60/59)

1c2fef7

dhpiggott force-pushed the support-parsing-timestamps-without-seconds branch from ec81b2c to 1c2fef7 Compare

May 30, 2024 16:21

dhpiggott commented

View reviewed changes

modules/core/src-jvm-native/smithy4s/Timestamp.scala Outdated Show resolved Hide resolved

Contributor Author

dhpiggott commented May 30, 2024

Hello @plokhotnyuk, I believe you're the expert in this area - would you mind taking a look at this change?

kubukoz approved these changes

View reviewed changes

Baccata requested changes

View reviewed changes

modules/core/src-js/smithy4s/Timestamp.scala Show resolved Hide resolved

dhpiggott added 3 commits

June 3, 2024 12:35


          Minimise the change

a161b9e


          scalafmt

54c0c5d


          Merge branch 'series/0.18' into support-parsing-timestamps-without-se…

e4946fc

…conds

Baccata approved these changes

View reviewed changes

Baccata merged commit e3572f3 into series/0.18

11 checks passed

dhpiggott deleted the support-parsing-timestamps-without-seconds branch

June 3, 2024 15:42

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet