Skip to content
This repository has been archived by the owner on Feb 1, 2019. It is now read-only.

Blob serialization 6 #73

Merged
merged 9 commits into from
May 21, 2018

Conversation

jamesray1
Copy link
Member

@jamesray1 jamesray1 commented May 16, 2018

Add serialization for blobs into collation bodies, with 2 new tests, and all tests passing. Plus minor changes e.g. to build.rs (refactoring all of main() to make_uml_diagram(), and adding crate-wide allow guards to minimize lots of warnings (allow I have been handling some, and more could be). Plus has changes from #67. We can merge #67 first, then this after rebasing on top of the new develop branch (after merging #67) (which will then be easier to review).

The UML diagram generated from cargo make uml-default-recommended via the terminal doesn't show a Body struct box with a chunks field. The diagram also doesn't display in the browser when generated from cargo make docs.

jamesray1 added 2 commits May 15, 2018 22:38
…rgo test and cargo build to pass. Failed to run custom build command for node, process didn't exit successfully.
…and all tests passing. Plus minor changes e.g. to build.rs, and adding crate-wide allow guards.
@jamesray1 jamesray1 closed this May 16, 2018
@jamesray1
Copy link
Member Author

Closing this because it should be better to review this as a PR to blob-serialization-v-5.

@jamesray1 jamesray1 reopened this May 16, 2018
@jamesray1
Copy link
Member Author

jamesray1 commented May 16, 2018

Reopening because this branch is behind blob-serialization-v-5, so we can merge #67 first, then this after rebasing on top of the new develop branch (after merging #67).

@jamesray1 jamesray1 merged commit a3451c3 into Drops-of-Diamond:develop May 21, 2018
@ltfschoen
Copy link
Collaborator

ltfschoen commented May 21, 2018

  • Why was this PR merged into the 'develop' branch without being reviewed by anyone?
  • Why are you committing log files of the test results? In this commit you've removed the log files that you generated but you only removed the ones that you generated in the node/ directory a038d90, but you didn't remove the log files that you generated in the root directory (i.e. mb_blob_to_collation_body4.log, mb_blob_zeros_to_collation_body3.log, mb_blob_zeros_to_collation_body4.log)?
  • The commit 7d78a07 in this PR that has a message saying that it fixes a merge conflict with ml.svg actually does not fully resolve the merge conflict that it created and I suspect actually corrupts the file (the text >> Temporary merge branch 2 has not been removed) from working anymore 7d78a07. I've created this PR to resolve the issue fix: Fix unresolved merge conflict #76
  • We need all PR's to be peer reviewed to avoid an accumulation of commits that may pollute the codebase.

@jamesray1
Copy link
Member Author

jamesray1 commented May 21, 2018

I did request a review for #67 and #73, https://gitter.im/Drops-of-Diamond/Development?at=5affd1dad245fe2eb7c1b6c8. Although those changes were introduced today while I was fixing up #73. Given the lack of a response I thought nobody was interested; perhaps I should've given more time.

you didn't remove the log files that you generated in the root directory

My mistake, fixed that with #77.

Merged #76.

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