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.

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

[ADP-3148] Add json to value parser #17

Merged
merged 8 commits into from
Sep 7, 2023
Merged

[ADP-3148] Add json to value parser #17

merged 8 commits into from
Sep 7, 2023

Conversation

paolino
Copy link
Contributor

@paolino paolino commented Sep 1, 2023 •
edited
Loading

  • Move Export.OpenAPI.Value to Export.OpenAPI.Value.ToJSON
  • Add Export.OpenAPI.Value.FromJSON
  • Add roundtrip tests between finetype Value and JSON Value

ADP-3148

@paolino paolino force-pushed the paolino/3148/json-value-parser branch from 9bc4f73 to 32c2b4a Compare September 1, 2023 15:08
Base automatically changed from paolino/3150/add-support-for-constrained-typs to main September 1, 2023 15:12
@paolino paolino force-pushed the paolino/3148/json-value-parser branch 3 times, most recently from 4c7015e to 4ff5a95 Compare September 4, 2023 14:38
@paolino paolino marked this pull request as ready for review September 4, 2023 14:39
@paolino paolino force-pushed the paolino/3148/json-value-parser branch 2 times, most recently from 7f17cec to 4011d83 Compare September 6, 2023 11:37
@paolino paolino force-pushed the paolino/3148/json-value-parser branch from 4011d83 to 02e86e7 Compare September 6, 2023 13:49
HeinrichApfelmus
HeinrichApfelmus reviewed Sep 6, 2023
View reviewed changes
Copy link
Collaborator

@HeinrichApfelmus HeinrichApfelmus left a comment

Choose a reason for hiding this comment

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

Excellent work, thank you! 😊 A couple of comments, though. Importantly, could you add validation for Integer and Natural as well? — it probably just got lost.

lib/fine-types/src/Language/FineTypes/Export/OpenAPI/Value/FromJSON.hs Outdated Show resolved Hide resolved
lib/fine-types/src/Language/FineTypes/Export/OpenAPI/Value/FromJSON.hs Outdated Show resolved Hide resolved
lib/fine-types/src/Language/FineTypes/Export/OpenAPI/Value/FromJSON.hs Outdated Show resolved Hide resolved
lib/fine-types/fine-types.cabal Outdated Show resolved Hide resolved
lib/fine-types/src/Language/FineTypes/Export/OpenAPI/Value/FromJSON.hs Outdated Show resolved Hide resolved
lib/fine-types/src/Language/FineTypes/Export/OpenAPI/Value/ToJSON.hs Outdated Show resolved Hide resolved
lib/fine-types/src/Language/FineTypes/Export/OpenAPI/Value/ToJSON.hs Outdated Show resolved Hide resolved
lib/fine-types/src/Language/FineTypes/Export/OpenAPI/Value/ToJSON.hs Outdated Show resolved Hide resolved
lib/fine-types/src/Language/FineTypes/Export/OpenAPI/Value/ToJSON.hs Outdated Show resolved Hide resolved
lib/fine-types/fine-types.cabal Outdated Show resolved Hide resolved
@paolino paolino force-pushed the paolino/3148/json-value-parser branch 2 times, most recently from 8443efc to 99b5da0 Compare September 7, 2023 12:33
HeinrichApfelmus
HeinrichApfelmus approved these changes Sep 7, 2023
View reviewed changes
Copy link
Collaborator

@HeinrichApfelmus HeinrichApfelmus left a comment

Choose a reason for hiding this comment

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

Excellent, thank you! 🤩 Except for a minor issue with a duplicated comment; I'm happy to see this merged!

lib/fine-types/src/Language/FineTypes/Export/OpenAPI/Value/FromJSON.hs Outdated Show resolved Hide resolved
@paolino paolino force-pushed the paolino/3148/json-value-parser branch from 99b5da0 to f3c50b2 Compare September 7, 2023 12:42
@paolino paolino added this pull request to the merge queue Sep 7, 2023
Merged via the queue into main with commit b6055a4 Sep 7, 2023
15 checks passed
@paolino paolino deleted the paolino/3148/json-value-parser branch September 7, 2023 14:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@HeinrichApfelmus HeinrichApfelmus HeinrichApfelmus approved these changes

Assignees

@paolino paolino

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@paolino @HeinrichApfelmus