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

Add serialisation server module #191

Merged
merged 18 commits into from Oct 27, 2021
Merged

Add serialisation server module #191

merged 18 commits into from Oct 27, 2021

Conversation

meatballs
Copy link
Member

@meatballs meatballs commented Oct 26, 2021 •
edited

  • Add server module
  • Write docs

@meatballs meatballs requested a review from s-cork October 26, 2021 13:49
@meatballs meatballs linked an issue Oct 26, 2021 that may be closed by this pull request
Data Tables Serialiser #140
Closed
@meatballs meatballs marked this pull request as ready for review October 26, 2021 16:34
s-cork
s-cork approved these changes Oct 27, 2021
View reviewed changes
Copy link
Collaborator

@s-cork s-cork left a comment

Choose a reason for hiding this comment

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

lgtm - just some questions and comments. Feel free to ignore.

docs/guides/modules/serialisation.rst Outdated Show resolved Hide resolved
docs/guides/modules/serialisation.rst Outdated Show resolved Hide resolved
docs/guides/modules/serialisation.rst Outdated Show resolved Hide resolved
docs/guides/modules/serialisation.rst Outdated Show resolved Hide resolved
server_code/serialisation.py Outdated Show resolved Hide resolved
docs/guides/modules/serialisation.rst Outdated Show resolved Hide resolved
docs/guides/modules/serialisation.rst Show resolved Hide resolved
docs/guides/modules/serialisation.rst Outdated Show resolved Hide resolved
@meatballs
Copy link
Member Author

All good comments. The docs were done in (too much of) a hurry!

@meatballs
Copy link
Member Author

I think it would be better to return a Schema instance from the server function, rather than the class. What do you reckon?

@meatballs
Copy link
Member Author

A few additions - I'm done with this now. Just let me know if you're happy with those or not.

@meatballs meatballs merged commit aff64cd into main Oct 27, 2021
@meatballs meatballs deleted the serialiser branch October 27, 2021 13:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@s-cork s-cork s-cork approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Data Tables Serialiser
2 participants
@meatballs @s-cork