cleanup stratum errors#10884
Conversation
debris
added
A0-pleasereview 🤓
|
What's the motivation behind replacing |
|
Getting rid of bloated error types which contain the same kind of errors on multiple levels and are never matched. I believe that if the 'error' is created only in one place and is never matched it does not deserve it's own type |
| } | ||
|
|
||
| fn push_work_all(&self, payload: String, tcp_dispatcher: &Dispatcher) -> Result<(), Error> { | ||
| fn push_work_all(&self, payload: String, tcp_dispatcher: &Dispatcher) { |
There was a problem hiding this comment.
wow, so this method has actually always returned an Ok(()), 👍
ordian
added
A8-looksgood 🦄
tomusdrw
left a comment
tomusdrw
left a comment
There was a problem hiding this comment.
IMHO Stringifying errors is completely backwards, I don't understand how this cleans things up. By looking at the error enumeration you could at least roughly identify negative cases which gives a tremendous insight into the code.
I don't see this change as an improvement at all, I think that rather we should eradicate ALL String error types in the codebase.
debris
added
A4-gotissues 💥
|
I'm not sure I agree with this either, in particular I think that a proper error type helps readability – at a glance I know what the anticipated error conditions for a crate/module are. New devs looking at the code are going to go look for the errors too, and if they have to chase around for strings sprinkled all over the place they'll hate us. @debris when you say that the error types are bloated, what are you referring to? You mean actually bloated as in "a potential performance problem" or more "unneeded type wankery"? |
Salvage some code from #10884 + some cleanup and typos.
* Cleanup stratum a bit Salvage some code from #10884 + some cleanup and typos. * HashSet::new does not allocate before first insert * Remove unused method push_work()
|
Closing as stale. |
* Cleanup stratum a bit Salvage some code from #10884 + some cleanup and typos. * HashSet::new does not allocate before first insert * Remove unused method push_work()
* Cleanup stratum a bit Salvage some code from #10884 + some cleanup and typos. * HashSet::new does not allocate before first insert * Remove unused method push_work()
4 participants
No description provided.