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

Save block reference in the queue on notification #1501

Merged
merged 1 commit into from Jul 1, 2016
Merged

Conversation

arkpar
Copy link
Collaborator

@arkpar arkpar commented Jun 30, 2016

No description provided.

@arkpar arkpar added the A0-pleasereview 🤓 Pull request needs code review. label Jun 30, 2016
@arkpar arkpar added this to the 1.2.1 milestone Jun 30, 2016
@rphmeier rphmeier added A8-looksgood 🦄 Pull request is reviewed well. and removed A0-pleasereview 🤓 Pull request needs code review. labels Jun 30, 2016
@@ -263,6 +263,9 @@ impl Miner {
let number = block.block().fields().header.number();
let difficulty = *block.block().fields().header.difficulty();
sealing_work.push(block);
if self.work_poster.is_some() {
Copy link
Contributor

Choose a reason for hiding this comment

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

Would have preferred to see it closer to the code that relies on it (i.e. the last line of the function).

Good enough for now, but perhaps add a comment saying what it's doing.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Placed it there to avoid locking the sealing_work mutex again. Notification is done with the mutex released

@gavofyork gavofyork added A6-mustntgrumble 💦 Pull request has areas for improvement. The author need not address them before merging. and removed A8-looksgood 🦄 Pull request is reviewed well. labels Jun 30, 2016
@arkpar
Copy link
Collaborator Author

arkpar commented Jun 30, 2016

Added a comment

@arkpar arkpar added A0-pleasereview 🤓 Pull request needs code review. and removed A6-mustntgrumble 💦 Pull request has areas for improvement. The author need not address them before merging. labels Jun 30, 2016
@gavofyork gavofyork merged commit d91e8cc into master Jul 1, 2016
@gavofyork gavofyork deleted the work-notify branch July 1, 2016 18:38
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
A0-pleasereview 🤓 Pull request needs code review.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants