refactor: Use ChainType enum exhaustively#27611
Conversation
|
The following sections might be updated with supplementary metadata relevant to reviewers and maintainers. ReviewsSee the guideline for information on the review process.
If your review is incorrectly listed, please react with 👎 to this comment and the bot will ignore it on the next update. |
|
Can remove |
Do you mean these sedited@0e7abdc ? |
|
I mean the ones of the touched methods |
|
Example: diff --git a/src/chainparams.h b/src/chainparams.h
index 6a65f40f80..23b272cc41 100644
--- a/src/chainparams.h
+++ b/src/chainparams.h
@@ -26,7 +26,6 @@ class ArgsManager;
/**
* Creates and returns a std::unique_ptr<CChainParams> of the chosen chain.
* @returns a CChainParams* of the chosen chain.
- * @throws a std::runtime_error if the chain is not supported.
*/
std::unique_ptr<const CChainParams> CreateChainParams(const ArgsManager& args, const ChainType chain);
@@ -38,7 +37,6 @@ const CChainParams &Params();
/**
* Sets the params returned by Params() to those for the given chain name.
- * @throws std::runtime_error when the chain is not supported.
*/
void SelectParams(const ChainType chain);
|
|
Updated d0767a9 -> ef39bdf (followUp27491_0 -> followUp27491_1, compare)
|
maflcko
left a comment
There was a problem hiding this comment.
lgtm, found another nit, feel free to ignore
This is a follow up of bitcoin#27491, more concretely bitcoin#27491 (comment), for not using default cases (as per the style guide), and bitcoin#27491 (comment) and bitcoin#27491 (comment) for avoiding dead code. Also change chain name to chain type in docstrings
|
Updated ef39bdf -> e230887 (followUp27491_1 -> followUp27491_2, compare)
|
|
Concept ACK. |
| case ChainType::MAIN: | ||
| return ""; | ||
| } | ||
| assert(false); |
There was a problem hiding this comment.
Our Developer Notes also suggest a comment // no default case, so the compiler can warn about missing cases.
e230887 refactor: Use ChainType enum exhaustively (TheCharlatan) Pull request description: This is a follow up of bitcoin#27491, more concretely bitcoin#27491 (comment), for not using default cases (as per the style guide), and bitcoin#27491 (comment) and bitcoin#27491 (comment) for avoiding dead code. ACKs for top commit: fanquake: ACK e230887 - deals with almost all follow up comments out of bitcoin#27491. Tree-SHA512: 1794190b03b91d3ca349a4da08e9610dbb3432983eee7cb21ecc758d1d7d710560c97661de14cdf493c28c00ebe8977511b4696055c0940e7f815b622dbacd16
This is a follow up of #27491, more concretely #27491 (comment), for not using default cases (as per the style guide), and #27491 (comment) and #27491 (comment) for avoiding dead code.