--
You received this message because you are subscribed to the Google Groups "cxx" group.
To unsubscribe from this group and stop receiving emails from it, send an email to cxx+uns...@chromium.org.
To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/cxx/CA%2BrzOEkmbeUgwixRAPQBeC2VBH2%3DbQ4F3rBinuSAzD2wfMRzZw%40mail.gmail.com.
Does modernize-make-shared work for scoped_refptr which IIUC chromium uses instead of shared_ptr?
Thoughts?
> I do wonder how you chose this particular mix of five; it's not consistently the most or least-impacting setMy feelings, basically. :)A side-focus of this thread is establishing/solidifying this whole 'adding new lints' process, so I picked things that I felt would be the least contentious lints to maximize room on the thread for folx to express more meta thoughts/suggestions. I also wanted at least one that's likely to be hit by a fair number of devs soon, so we could start exploring how to look at the 'Not Useful' metrics that're a thing here.Going smallest->largest would give us:- modernize-make-shared (1)- modernize-avoid-bind (3)- modernize-use-noexcept (22)- modernize-use-transparent-functors (53)- google-readability-namespace-comments (475)
--
You received this message because you are subscribed to the Google Groups "cxx" group.
To unsubscribe from this group and stop receiving emails from it, send an email to cxx+uns...@chromium.org.
To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/cxx/CA%2BrzOEkzQwemA7mQL02M2F-B3xpYuqmJhNdsSa3kOG03UJJrBg%40mail.gmail.com.
I... don't think Chromium has any process that involves style guide waivers,
and did notice 'wrapper' types being flagged by this rule. Depending on what folx here think about automated style nits like this, that may be a valid reason to drop this. I'm in the camp of "implicit conversions often do more harm than good," and users can always NOLINT their code, so if this does the right thing the strong majority of the time, I'm personally in favor of it.
Following up, here are the metrics I've collected for these lints since we enabled them last month:- modernize-loop-convert fired 301 times- modernize-make-shared fired 0 times
- readability-redundant-member-init fired 79 times
- google-explicit-constructor fired 128 times
- modernize-use-transparent-functors fired 9 times(where "fired" means "actually showed up as a robocomment on a CL.")For each of those, users clicked "not useful" the following number of times:- modernize-loop-convert -- 11 (3.6% of lints)
- modernize-make-shared -- 0 (0% of lints)
- readability-redundant-member-init -- 1 (1.2% of lints)
- google-explicit-constructor -- 4 (3.1% of lints)
- modernize-use-transparent-functors -- 0 (0% of lints)Since there aren't many "not useful"s here, I'll investigate the individual comments offline to see if any of these were the result of clang-tidy bugs.Even if not, I think these numbers are good enough that these lints should all stick.
Happy Wednesday, friends!Following up, here are the metrics I've collected for these lints since we enabled them last month:- modernize-loop-convert fired 301 times- modernize-make-shared fired 0 times
- readability-redundant-member-init fired 79 times
- google-explicit-constructor fired 128 times
- modernize-use-transparent-functors fired 9 times(where "fired" means "actually showed up as a robocomment on a CL.")For each of those, users clicked "not useful" the following number of times:- modernize-loop-convert -- 11 (3.6% of lints)
- modernize-make-shared -- 0 (0% of lints)
- readability-redundant-member-init -- 1 (1.2% of lints)
- google-explicit-constructor -- 4 (3.1% of lints)
- modernize-use-transparent-functors -- 0 (0% of lints)Since there aren't many "not useful"s here, I'll investigate the individual comments offline to see if any of these were the result of clang-tidy bugs.