Skip to content

Add methods for serialization #48

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
Closed
wants to merge 1 commit into from
Closed

Add methods for serialization #48

wants to merge 1 commit into from

Conversation

@UserUnknownFactor
Copy link

It would be nice to have the python library prepared for serialization functionality.

@KOLANICH
Copy link
Contributor

KOLANICH commented Aug 3, 2020

#13 and #12 are related

KOLANICH
KOLANICH reviewed Aug 3, 2020
View reviewed changes
KOLANICH
KOLANICH reviewed Aug 3, 2020
View reviewed changes
KOLANICH
KOLANICH reviewed Aug 3, 2020
View reviewed changes
KOLANICH
KOLANICH suggested changes Aug 3, 2020
View reviewed changes
Copy link
Contributor

@KOLANICH KOLANICH left a comment

Choose a reason for hiding this comment

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

In its current state it is inacceptable to be merged.

@UserUnknownFactor UserUnknownFactor force-pushed the master branch 2 times, most recently from 80a8585 to 3113da9 Compare August 3, 2020 13:44
KOLANICH
KOLANICH reviewed Aug 3, 2020
View reviewed changes
dgelessus
dgelessus reviewed Aug 3, 2020
View reviewed changes
@UserUnknownFactor
Copy link
Author

Example of code: #48 (comment)
Also do KaitaiStructs runtime needs __init__ mention the stuff?
Because currently compiler creates code that doesn't call super anyway.

@UserUnknownFactor
Copy link
Author

I removed all changes related to possible compiler implementation and alignment, I'll do them as a separate PRs.

generalmimon
generalmimon reviewed Aug 5, 2020
View reviewed changes
generalmimon
generalmimon reviewed Aug 5, 2020
View reviewed changes
generalmimon
generalmimon reviewed Aug 5, 2020
View reviewed changes
generalmimon
generalmimon requested changes Aug 5, 2020
View reviewed changes
Copy link
Member

@generalmimon generalmimon left a comment

Choose a reason for hiding this comment

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

I request some changes in the comments above.

@UserUnknownFactor UserUnknownFactor force-pushed the master branch 4 times, most recently from 3938615 to 24e5259 Compare August 7, 2020 14:05
@UserUnknownFactor
Copy link
Author

This should be enough, right?

generalmimon
generalmimon reviewed Aug 9, 2020
View reviewed changes
KOLANICH
KOLANICH reviewed Aug 10, 2020
View reviewed changes
KOLANICH
KOLANICH reviewed Aug 10, 2020
View reviewed changes
KOLANICH
KOLANICH reviewed Aug 10, 2020
View reviewed changes
@UserUnknownFactor UserUnknownFactor force-pushed the master branch 4 times, most recently from 656b27f to 787b148 Compare August 10, 2020 09:08
KOLANICH
KOLANICH reviewed Aug 10, 2020
View reviewed changes
generalmimon
generalmimon reviewed Aug 10, 2020
View reviewed changes
generalmimon
generalmimon reviewed Aug 11, 2020
View reviewed changes
generalmimon
generalmimon reviewed Aug 11, 2020
View reviewed changes
@generalmimon generalmimon mentioned this pull request May 20, 2021
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@dgelessus dgelessus dgelessus left review comments

@generalmimon generalmimon generalmimon requested changes

+1 more reviewer

@KOLANICH KOLANICH KOLANICH requested changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@UserUnknownFactor @KOLANICH @dgelessus @generalmimon