Derive Eq for all structs that derive PartialEq #1763
Conversation
| @@ -609,7 +609,7 @@ mod tests { | |||
|
|
|||
| const EVENT_DEADLINE: u64 = 5 * FRESHNESS_TIMER; | |||
|
|
|||
| #[derive(Clone, Eq, Hash, PartialEq)] | |||
| #[derive(Clone, Eq, Hash, PartialEq, Eq)] | |||
There was a problem hiding this comment.
This already derived Eq, no change needed here.
|
LGTM. Can you squash the second commit into the first so that its one commit? |
gcomte
force-pushed
the
feature/derive-eq
branch
from
90ef181 to
71d43b3
Compare
Done |
Codecov ReportBase: 90.79% // Head: 90.76% // Decreases project coverage by
Additional details and impacted files@@ Coverage Diff @@
## main #1763 +/- ##
==========================================
- Coverage 90.79% 90.76% -0.04%
==========================================
Files 87 87
Lines 46969 46969
Branches 46969 46969
==========================================
- Hits 42646 42631 -15
- Misses 4323 4338 +15
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. ☔ View full report at Codecov. |
| @@ -679,7 +679,7 @@ impl Readable for IrrevocablyResolvedHTLC { | |||
| /// tells you the last block hash which was block_connect()ed. You MUST rescan any blocks along | |||
| /// the "reorg path" (ie disconnecting blocks until you find a common ancestor from both the | |||
| /// returned block hash and the the current chain and then reconnecting blocks to get to the | |||
| /// best chain) upon deserializing the object! | |||
| /// best chain) upon deserializing the object | |||
There was a problem hiding this comment.
This change seems unrelated?
There was a problem hiding this comment.
Oh I'm sorry. Gonna remove
gcomte
force-pushed
the
feature/derive-eq
branch
2 times, most recently
from
091b06a to
d65c670
Compare
| @@ -824,6 +824,7 @@ pub type TransactionOutputs = (Txid, Vec<(u32, TxOut)>); | |||
| #[cfg(any(test, fuzzing, feature = "_test_utils"))] | |||
| /// Used only in testing and fuzzing to check serialization roundtrips don't change the underlying | |||
| /// object | |||
| impl<Signer: Sign> Eq for ChannelMonitor<Signer> {} | |||
There was a problem hiding this comment.
Oops, see the cfg above (here and the next hunk) - we were only implementing eq for test/fuzzing here, so we'll need to duplicate the cfg (or just not bother - its test-only code anyway).
There was a problem hiding this comment.
Added changes in a speperate commit for you to see: 4483baf
If legitimate, I'll squash the commits.
|
Probably best to just leave out the Otherwise, LGTM |
gcomte
force-pushed
the
feature/derive-eq
branch
from
4483baf to
aa916bb
Compare
|
Squashed the commits ✔️ |
Fixes #1762
I'm not entirely sure about the legitimacy of this commit but according to the Rust docs, deriving
Eqfor a struct makes sure that all of it's fields areEqas well:If I understand this premise correctly, this commit should not introduce any equivalence false positives.