Skip to content

Minor fixes for 0.7.0#59

Merged
rphmeier merged 6 commits intomasterfrom
andre/fix-commit-callback
Apr 5, 2019
Merged

Minor fixes for 0.7.0#59
rphmeier merged 6 commits intomasterfrom
andre/fix-commit-callback

Conversation

@andresilva
Copy link
Copy Markdown
Contributor

  • CommunicationOut and CommunicationIn are no longer encodable/decodable. The commit in CommunicationOut takes a callback that is not serializable. In general, these messages won't need to be encoded anyway.
  • Removed the Clone on callback, seemed dangerous and it's only needed for tests.
  • Removed unused file.
  • Make fields of CommitValidationResult public.

src/lib.rs Outdated
pub num_precommits: usize,
pub num_duplicated_precommits: usize,
pub num_equivocations: usize,
pub num_invalid_voters: usize,
Copy link
Copy Markdown
Contributor

@rphmeier rphmeier Apr 3, 2019

Choose a reason for hiding this comment

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

Prefer getters since then we can add more without breaking API or add a _priv field. Doc-strings on the fields would be helpful also.

Copy link
Copy Markdown

@marcio-diaz marcio-diaz left a comment

Choose a reason for hiding this comment

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

lgtm

Work(Box<FnMut(CommitProcessingOutcome) + Send>),
}

#[cfg(test)]
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Thanks.

@rphmeier rphmeier merged commit ac853e7 into master Apr 5, 2019
@rphmeier rphmeier deleted the andre/fix-commit-callback branch April 5, 2019 20:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants