Skip to content
This repository has been archived by the owner on Nov 6, 2020. It is now read-only.

Fix JournalDB era marker #690

Merged
merged 4 commits into from Mar 13, 2016
Merged

Fix JournalDB era marker #690

merged 4 commits into from Mar 13, 2016

Conversation

arkpar
Copy link
Collaborator

@arkpar arkpar commented Mar 12, 2016

No description provided.

@arkpar arkpar added the A0-pleasereview 🤓 Pull request needs code review. label Mar 12, 2016
@arkpar arkpar changed the title Prevent overwriting latest era marker with older value Fix JournalDB era marker Mar 12, 2016
@@ -435,7 +441,10 @@ impl JournalDB for EarlyMergeDB {
trace!(target: "jdb.ops", " Deletes: {:?}", removes);
}
try!(batch.put(&last, r.as_raw()));
try!(batch.put(&LATEST_ERA_KEY, &encode(&now)));
if now >= self.latest_era {
Copy link
Contributor

Choose a reason for hiding this comment

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

not just >?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Can't use 0 as special value with >. Changed to Option

@gavofyork gavofyork added A8-looksgood 🦄 Pull request is reviewed well. and removed A0-pleasereview 🤓 Pull request needs code review. labels Mar 12, 2016
@gavofyork
Copy link
Contributor

looks good other than the question.

@arkpar arkpar added A0-pleasereview 🤓 Pull request needs code review. and removed A8-looksgood 🦄 Pull request is reviewed well. labels Mar 12, 2016
@gavofyork
Copy link
Contributor

weren't there tests failing with this? do they pass now?

@gavofyork gavofyork added A8-looksgood 🦄 Pull request is reviewed well. A3-inprogress ⏳ Pull request is in progress. No review needed at this stage. and removed A0-pleasereview 🤓 Pull request needs code review. labels Mar 12, 2016
@gavofyork
Copy link
Contributor

tests todo.

@arkpar
Copy link
Collaborator Author

arkpar commented Mar 13, 2016

Added a test

@arkpar arkpar removed the A3-inprogress ⏳ Pull request is in progress. No review needed at this stage. label Mar 13, 2016
jdb.remove(&bar);
jdb.commit(0, &b"0b".sha3(), None).unwrap();
assert!(jdb.can_reconstruct_refs());
jdb.commit(2, &b"1".sha3(), Some((1, b"1".sha3()))).unwrap();
Copy link
Contributor

Choose a reason for hiding this comment

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

should be commit(2, &b"2".sha3(), ...

gavofyork added a commit that referenced this pull request Mar 13, 2016
Fix JournalDB era marker
@gavofyork gavofyork merged commit 4e8092b into master Mar 13, 2016
@gavofyork gavofyork deleted the fixjdb branch March 13, 2016 12:11
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
A8-looksgood 🦄 Pull request is reviewed well.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants