fixed #13129 - apply --std with --clang / fixed usage of Standards::stdValue#6742
Conversation
This comment was marked as outdated.
This comment was marked as outdated.
|
"fun" fact: I found part of this in a 2.5 year old local changelist... |
firewave
force-pushed
the
stdvalue
branch
3 times, most recently
from
b9240dc to
2b1e751
Compare
firewave
force-pushed
the
stdvalue
branch
5 times, most recently
from
4149fc7 to
e8590cd
Compare
firewave
changed the title
Standards::stdValue--std with --clang / fixed usage of Standards::stdValue
|
CC @ludviggunne please also take a look at what you made me do 😉 |
|
Sorry! |
I was asking for a review not pleasantries. 😁 |
|
Looks good to me. |
|
Thanks! |
| std::string stdValueCPP; | ||
|
|
||
| bool setC(std::string str) { | ||
| stdValue = str; |
There was a problem hiding this comment.
I just wonder if setC and setCPP are needed.
If only setStd is called from outside we could implement all the logic in that directly or make setC and setCPP private? I feel maybe the code would be a bit simpler but on the other hand testability is important too.
There was a problem hiding this comment.
That should be detected by the CI then. Maybe there is a conflict. Will take a look later. And more changes are coming and will integrate that cleanup if possible.
danmar
left a comment
danmar
left a comment
There was a problem hiding this comment.
I see nothing wrong here.. so feel free to merge. I don't have a strong opinion about setC/setCPP, feel free to make that decision.
3 participants
No description provided.