Skip to content
This repository was archived by the owner on Jan 31, 2025. It is now read-only.

Conversation

amikai
Copy link
Member

@amikai amikai commented Aug 21, 2017

This PR is add a feature which describes in #12.
The three thing i do in this PR:

  1. Implement the serializer interface.
  2. Implement the json format serializer.
  3. Write unit test for above.

@iankuan
Copy link
Contributor

iankuan commented Oct 1, 2017

It was almost done, isn't it ? Just give me some unittests.

However, you could read this material, How to write a good commit message(Chinese). I'm not saying you did a bad job. Just have something you can improve.

@iankuan iankuan mentioned this pull request Oct 4, 2017
@iankuan
Copy link
Contributor

iankuan commented Oct 4, 2017

@as23041248 , please rebase your branch. We import many features like CI and Makefile, which can test your code automatically.

@iankuan iankuan self-requested a review October 5, 2017 07:25
Copy link
Contributor

@iankuan iankuan left a comment

Choose a reason for hiding this comment

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

Go ahead!

Copy link
Member Author

@amikai amikai left a comment

Choose a reason for hiding this comment

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

YOYO
test

@amikai amikai requested a review from iankuan October 5, 2017 07:35
@amikai amikai force-pushed the feature/serialization branch from e29ae6e to 9a682b6 Compare October 5, 2017 20:42
Copy link
Contributor

@iankuan iankuan left a comment

Choose a reason for hiding this comment

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

Seems great. By the way, I notice that this branch feature/serialization doesn't catch up mainline. Please rebasing it after few minutes.

@amikai amikai force-pushed the feature/serialization branch 2 times, most recently from fd0b9d1 to 4d1f642 Compare October 6, 2017 15:45
Copy link
Contributor

@iankuan iankuan left a comment

Choose a reason for hiding this comment

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

You didn't rebase completely. Did you try pull XXX --rebase? The three of last shouldn't appear in your commit msg.

@amikai amikai requested a review from iankuan October 7, 2017 02:28
Copy link
Contributor

@iankuan iankuan left a comment

Choose a reason for hiding this comment

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

No fixup.

@amikai amikai force-pushed the feature/serialization branch 2 times, most recently from 7a18341 to 3c2c250 Compare October 7, 2017 02:44
@amikai amikai requested review from iankuan and removed request for iankuan October 7, 2017 02:44
amikai added 2 commits October 7, 2017 11:01
The serializer in Json convert python dict to string(raw data),
and deserializer convert string(raw data) to python dict.
@amikai amikai force-pushed the feature/serialization branch from 3c2c250 to 884ea53 Compare October 7, 2017 03:17
@amikai amikai requested a review from iankuan October 7, 2017 03:19
Copy link
Contributor

@iankuan iankuan left a comment

Choose a reason for hiding this comment

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

Meld last two commits into one. Hint: "rebase -i" and then "-f fixup".

Test json serializer by converting between python dict and raw json
data.
@amikai amikai force-pushed the feature/serialization branch from 884ea53 to 22acb5c Compare October 7, 2017 05:42
@amikai amikai changed the title [WIP] Impl serializer base class. Impl serializer interface and json serializer Oct 7, 2017
@iankuan iankuan merged commit 2c1e73a into USCC-LAB:feature/serialization Oct 8, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants