mit-dci / opencbdc-tx

A transaction processor for a hypothetical, general-purpose, central bank digital currency
Other
901 stars 199 forks source link

Add unit test to cover non-error cases of cbdc::nuraft_serializer::read() #118

Closed ykaravas closed 2 years ago

ykaravas commented 2 years ago

This pull request addresses https://github.com/mit-dci/opencbdc-tx/issues/117#issue-1257715247

Added a unit test to cover non error case for cbdc::nuraft_serializer::read()

Closes https://github.com/mit-dci/opencbdc-tx/issues/117

HalosGhost commented 2 years ago

@ykaravas this looks simple and solid to me (adding a happy-path, round-trip test case for the raft serialization). Can you squash to a single commit and I'll merge?

ykaravas commented 2 years ago

Hello @HalosGhost I seem to be having trouble squashing the commits. It is not a practice i am used to doing and having the commits merged in from changes to trunk interspersed with mine is confusing me a bit as to how i would accomplish this. Namely, i am wondering how to deal with commits interspersed with mine from merging trunk into my branch along the way. I think my mistake was merging trunk into my branch along the way instead of rebasing. Should i simply create new branch with my changes, and in turn, a new pull request? Please advise.

HalosGhost commented 2 years ago

@ykaravas it's a bit new for many; no worries! The easiest way to handle newly-added commits upstream is (assuming you have this fork as a remote locally named upstream) git pull --rebase upstream trunk. That will replay all your local commits on top of trunk, and should put you in a good place. After that git rebase -i for an interactive rebase (appropriately choosing squash or fixup to get to a single commit) should have everything cleaned up!

ykaravas commented 2 years ago

@HalosGhost Awesome thank you! I had to do a force push to get it to update, however; is that normal? If so, it should be all set now.

HalosGhost commented 2 years ago

I had to do a force push to get it to update, however; is that normal?

Git is very cautious about allowing history-rewriting (which is what rebasing is), so yeah; after rebasing, you'll always need to force-push.