-
Notifications
You must be signed in to change notification settings - Fork 1.5k
Reodered ErrorMessage constructor parameter message and ID parameters #7790
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
I made sure that I covered all the usages of these constructors by forcing compilation errors (normal compilation, compiling the UTs as well). |
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't really see the advantage with this reordering. It looks rather dangerous. I don't understand how you made sure that all ids and messages are reordered - you said something about a compiler error.
I appreciate you try to fix something here.. would you like some suggestions for bugs that you can look at?
I was just fixing the TODO that is in the code. The main advantage is simply to give consistency to the order of the msg and id fields in the multiple constructors of ErrorMessage. Regarding the compiler error, I was just mentioning that in order to find all applicable instances of the constructor, I forced a compiler error to happen specifically in these constructors that I updated. Then I updated the parameters in all of them and then I reverted the change that triggered the compiler error. Sure I can take suggestions for bugs to look at! |
sorry for slow reply. I was at cppcon last week and was extra busy before it..
Thanks! How about this one: https://trac.cppcheck.net/ticket/14126 If you can make sure that a The |
No description provided.