Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix Travis Build #16

Merged
merged 10 commits into from
May 18, 2017
Merged

Fix Travis Build #16

merged 10 commits into from
May 18, 2017

Conversation

slowli
Copy link
Contributor

@slowli slowli commented May 17, 2017

Currently, the Travis build for the project fails for multiple reasons. This PR tries to fix that.

Copy link
Contributor

@alexauroradev alexauroradev left a comment

Choose a reason for hiding this comment

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

LGTM, except Leader Election

1. *desired* The algorithm should not give preferences for any nodes (or artificially decrease priority for other nodes).
In our case, we give preferences for nodes that were less involved in last `F` blocks. Perhaps such nodes do not give block proposals due to objective reasons: bad connection, server overloading, etc.
2. *desired* Round Robin orders could be calculated strictly after the previous block was accepted.
# Leader Election
Copy link
Contributor

Choose a reason for hiding this comment

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

Why have you deleted the Leader Election part?
It's the file that was in repo since e238408

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I can restore it, but frankly speaking, it would not pass my review if it was being pulled into the repo right now. I would prefer to merge it in a separate PR with @VukW . To be honest, anchoring spec is only marginally better, but it's slightly more important, too.

Copy link
Contributor

Choose a reason for hiding this comment

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

Ok

alexauroradev
alexauroradev previously approved these changes May 18, 2017
Copy link
Contributor

@alexauroradev alexauroradev left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Contributor

@alexauroradev alexauroradev left a comment

Choose a reason for hiding this comment

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

LGTM

@slowli slowli merged commit 82f5fbf into exonum:master May 18, 2017
@slowli slowli deleted the fix-travis-build branch May 18, 2017 11:13
This was referenced May 18, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants