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

Add a first iteration containing documentation of design. #257

Merged
merged 9 commits into from
Jun 27, 2022

Conversation

Baccata
Copy link
Contributor

@Baccata Baccata commented Jun 7, 2022

Specifically, this iteration focuses on explaining the rationale behind the Schema construct, and provides an explanation for a subset of the possible schemas

Baccata and others added 2 commits June 7, 2022 16:43
Specifically, this iteration focuses on explaining the rationale behind
the Schema construct, and provides an explanation for a subset of the
possible schemas
Copy link
Member

@kubukoz kubukoz left a comment

Choose a reason for hiding this comment

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

I was editing this in the IDE so I don't have proper github suggestions, enjoy a commit instead:

https://github.com/disneystreaming/smithy4s/compare/design-documentation...kubukoz:design-documentation-review-jk?expand=1

Added a lot of newlines so that future diffs look less cluttered (shorter lines, and it renders the same in the HTML). Also my editor likes to trim trailing whitespace on lines so that also affects the size of the diff...

@@ -0,0 +1,14 @@
---
Copy link
Member

Choose a reason for hiding this comment

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

dunno if it's a file naming thing, but it'd look better as "Design" (uppercase D)

image

Copy link
Contributor

@lewisjkl lewisjkl left a comment

Choose a reason for hiding this comment

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

I didn't quite get through it all (will continue tomorrow), but so far the content is really awesome! I noted a few grammar errors/improvements. Feel free to take or leave them as you see fit.


Indeed, for each field, there is an associated reference to a schema (int, string, ...), a string label, and a lambda calling the case class accessor that allows the retrieval of the associated field value. Additionally, the constructor of the case class is also referenced in the Schema.

Typically, the accessors are needed for encoding the data, which involves destructuring it to access its individual components. The labels are there to cater to serialisation mechanisms like Json or Xml, which apply some labelled nesting on component data.
Copy link
Contributor

Choose a reason for hiding this comment

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

which apply some labelled nesting on component data

I'm not quite clear what is meant by this

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I've rephrased, let me know how it reads

Baccata and others added 2 commits June 8, 2022 09:41
@disneystreaming disneystreaming deleted a comment from lewisjkl Jun 8, 2022
@disneystreaming disneystreaming deleted a comment from lewisjkl Jun 8, 2022
@disneystreaming disneystreaming deleted a comment from lewisjkl Jun 8, 2022
@disneystreaming disneystreaming deleted a comment from lewisjkl Jun 8, 2022
@disneystreaming disneystreaming deleted a comment from lewisjkl Jun 8, 2022
@disneystreaming disneystreaming deleted a comment from lewisjkl Jun 8, 2022
@disneystreaming disneystreaming deleted a comment from lewisjkl Jun 8, 2022
@disneystreaming disneystreaming deleted a comment from lewisjkl Jun 8, 2022
@disneystreaming disneystreaming deleted a comment from lewisjkl Jun 8, 2022
@Baccata
Copy link
Contributor Author

Baccata commented Jun 8, 2022

@lewisjkl since Jakub had provided a commit that impacted your suggestions, I've taken the liberty of removing your comments containing only amendments, after making sure they were addressed.

@disneystreaming disneystreaming deleted a comment from lewisjkl Jun 9, 2022
@disneystreaming disneystreaming deleted a comment from lewisjkl Jun 9, 2022
@disneystreaming disneystreaming deleted a comment from lewisjkl Jun 9, 2022
@Baccata Baccata merged commit eee0353 into main Jun 27, 2022
@Baccata Baccata deleted the design-documentation branch June 27, 2022 13:54
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