Add utility to convert bool to string |
|||
Issue descriptionThere are several hundred places in the codebase that do something like x ? "true" : "false" Often this happens when printing out or serializing booleans. We should consider adding a little bool-to-string (more precisely, to const char*) conversion helper somewhere and using it in all these places. Might save binary size too.
,
Mar 31 2017
,
Mar 31 2017
Base string library is not a right place to keep its stability. It is better to adopt this simple utility module-by-module. Which modules do you have in mind? I will try to tackle those modules first.
,
Mar 31 2017
I didn't have particular modules in mind. Codesearch turned up several hundred cases scattered across a variety of places. This may or may not be suitable for base. If it's not, it's somewhat questionable where it is suitable for.
,
Mar 31 2017
Definitely not for base - base is meant to be the utter necessity. This is not the case to my view. More importantly the OWNERS of the base won't allow the check-in. I myself will look which might be a good start. This will be an incremental effort.
,
Mar 31 2017
Have you actually asked the base/ OWNERS? There's a high bar for getting things into base/ certainly, but we have a lot of other utilities there when they're broadly useful. Hundreds of uses scattered widely across the entire codebase would meet that bar. Last time brettw wrote on this I recall him urging people not to move things into base/ on the hope it would be useful elsewhere, or when there was a single higher-level spot that would work. But in this case I'm skeptical those caveats apply. I mean, do whatever cleanup you want, I just think you're saying "definitely not" about things that are, at best, not definite. And I'd hate to add six different identical conversion helpers in brand-new utility files in subdirectories, which seems like a cure worse than the disease in some sense.
,
Aug 24 2017
Back in March we exchanged discussions with the owners. We did not measure the amount of savings in the image size. However considering that the target ChromeOS devices do have ample disk space and the memory, the scope of the code change does not look worthwhile to bring in. I am marking this as WONTFIX after long radio silence. If this is not agreeable, please reopen. |
|||
►
Sign in to add a comment |
|||
Comment 1 by pkasting@chromium.org
, Mar 29 2017