New issue
Advanced search Search tips

Issue 706162 link

Starred by 2 users

Issue metadata

Status: WontFix
Owner: ----
Closed: Aug 2017
Components:
EstimatedDays: ----
NextAction: ----
OS: All
Pri: 3
Type: Task



Sign in to add a comment

Add utility to convert bool to string

Project Member Reported by pkasting@chromium.org, Mar 28 2017

Issue description

There 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.
 
Labels: Hotlist-CodeHealth

Comment 2 by porce@chromium.org, Mar 31 2017

Owner: porce@chromium.org
Status: Assigned (was: Available)

Comment 3 by porce@chromium.org, 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.
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.

Comment 5 by porce@chromium.org, 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.
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.

Comment 7 by porce@chromium.org, Aug 24 2017

Owner: ----
Status: WontFix (was: Assigned)
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