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

remove trait bounds from several structs #9055

Merged
merged 1 commit into from
Jul 9, 2018
Merged

remove trait bounds from several structs #9055

merged 1 commit into from
Jul 9, 2018

Conversation

debris
Copy link
Collaborator

@debris debris commented Jul 5, 2018

@debris debris added A0-pleasereview 🤓 Pull request needs code review. A2-insubstantial 👶 Pull request requires no code review (e.g., a sub-repository hash update). M4-core ⛓ Core client code / Rust. labels Jul 5, 2018
@5chdn 5chdn added this to the 2.0 milestone Jul 5, 2018
Copy link
Collaborator

@niklasad1 niklasad1 left a comment

Choose a reason for hiding this comment

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

Looks good to me

/// Construct new object with given `pre` and `post`.
pub fn new(pre: T, post: T) -> Self { if pre == post { Diff::Same } else { Diff::Changed(pre, post) } }
pub fn new(pre: T, post: T) -> Self where T: Eq {
Copy link
Collaborator

@niklasad1 niklasad1 Jul 6, 2018

Choose a reason for hiding this comment

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

I'm not that familiar with derive rules for Eq, but looks a little strange that we have to specify Eq in the constructor (T: Eq) when we have specified it in the derive already!

Can somebody explain this? :)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

derive(Debug) means

impl<T> Debug for Diff<T> where T: Debug {
  // ...
}

so it is a separate impl and has nothing to do with this one

@niklasad1 niklasad1 added A8-looksgood 🦄 Pull request is reviewed well. and removed A0-pleasereview 🤓 Pull request needs code review. labels Jul 6, 2018
@5chdn
Copy link
Contributor

5chdn commented Jul 7, 2018

Needs a 2nd review.

@andresilva andresilva merged commit c7d2184 into master Jul 9, 2018
@niklasad1 niklasad1 deleted the trait_bounds branch July 9, 2018 11:58
ordian added a commit to ordian/parity that referenced this pull request Jul 9, 2018
…rp_sync_on_light_client

* 'master' of https://github.com/paritytech/parity:
  remove trait bounds from several structs (openethereum#9055)
dvdplm added a commit that referenced this pull request Jul 10, 2018
* master:
  Clean up deprecated options and add CHECK macro (#9036)
  Replace `std::env::home_dir` with `dirs::home_dir` (#9077)
  fix warning in secret-store test (#9074)
  SeedHashCompute remove needless `new` impl (#9063)
  remove trait bounds from several structs (#9055)
  docs: add changelog for 1.10.9 stable and 1.11.6 beta (#9069)
  Enable test in `miner/pool/test` (#9072)
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
A2-insubstantial 👶 Pull request requires no code review (e.g., a sub-repository hash update). A8-looksgood 🦄 Pull request is reviewed well. M4-core ⛓ Core client code / Rust.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants