Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
1892473003:
C++11 doc cleanup (Closed)
|
Created:
4 years, 8 months ago by Peter Kasting Modified:
4 years, 8 months ago CC:
chromium-reviews, danakj+watch_chromium.org, jbroman+cpp_chromium.org Base URL:
https://chromium.googlesource.com/chromium/src.git@master Target Ref:
refs/pending/heads/master Project:
chromium Visibility:
Public. |
DescriptionC++11 doc cleanup:
Changes with a visible effect:
* Always comma-separate multiple items
* Generally avoid <br>s except where listing code
* Use the phrase "Google Style Guide", not "Google style guide", "C++ style guide", "Google Style Guide on XXX", etc.
* declval item was in the wrong section, written poorly, and had broken links
* Remove notes about MSVC2013 caveats
* Fix formatting of code (use correct number of spaces, etc.)
* Remove notes about FINAL and OVERRIDE macros, they're dead
* Slight clarification to nullptr instructions to make it clear that replacing NULL in existing code is OK
* Remove note forbidding general use of std::tuple since we explicitly allow general use of std::tuple
* Change comments like "Reevaluate once MSVS2015 is available" to "Reevaluate now that MSVS2015 is available"
* Add missing links to style guide for various banned items
* Various other tiny wording fixes/clarifications
Changes with no visible effect:
* One space after a period
* No line length limit (makes it easier to find an item to edit when scrolling through doc, matches how half of doc was already working, wrapping was very inconsistent otherwise)
* Collapse together sequential <code> blocks with no intervening text
BUG=none
TEST=none
Committed: https://crrev.com/07c0959a98ec440e930b40445b0038af9fc1eca9
Cr-Commit-Position: refs/heads/master@{#387445}
Patch Set 1 #
Total comments: 4
Patch Set 2 : Review comments #Messages
Total messages: 18 (9 generated)
Description was changed from ========== Cleanup BUG=none TEST=none ========== to ========== C++11 doc cleanup: Changes with a visible effect: * Always comma-separate multiple items * Generally avoid <br>s except where listing code * Use the phrase "Google Style Guide", not "Google style guide", "C++ style guide", "Google Style Guide on XXX", etc. * declval item was in the wrong section, written poorly, and had broken links * Remove notes about MSVC2013 caveats * Fix formatting of code (use correct number of spaces, etc.) * Remove notes about FINAL and OVERRIDE macros, they're dead * Slight clarification to nullptr instructions to make it clear that replacing NULL in existing code is OK * Remove note forbidding general use of std::tuple since we explicitly allow general use of std::tuple * Change comments like "Reevaluate once MSVS2015 is available" to "Reevaluate now that MSVS2015 is available" * Add missing links to style guide for various banned items * Various other tiny wording fixes/clarifications Changes with no visible effect: * One space after a period * No line length limit (makes it easier to find an item to edit when scrolling through doc, matches how half of doc was already working, wrapping was very inconsistent otherwise) * Collapse together sequential <code> blocks with no intervening text BUG=none TEST=none ==========
pkasting@chromium.org changed reviewers: + danakj@chromium.org
jbroman@chromium.org changed reviewers: + jbroman@chromium.org
lgtm
The CQ bit was checked by pkasting@chromium.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1892473003/1 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1892473003/1
Thanks you put a lot of work into this. LGTM. https://codereview.chromium.org/1892473003/diff/1/styleguide/c++/c++11.html File styleguide/c++/c++11.html (right): https://codereview.chromium.org/1892473003/diff/1/styleguide/c++/c++11.html#n... styleguide/c++/c++11.html:322: <td>Useful for fixed-size arrays. <a href="https://groups.google.com/a/chromium.org/d/topic/cxx/5iFNE8P5qT4/discussion">Discussion thread</a><br> Note that non-member <code>cbegin()</code> and <code>cend()</code> are not available until C++14.</td> <br/>? Line breaking after a br makes sense in the HTML too I think. https://codereview.chromium.org/1892473003/diff/1/styleguide/c++/c++11.html#n... styleguide/c++/c++11.html:436: <td><a href="https://crbug.com/554987">Tracking bug</a> to plan moving from <code>base::Tuple</code> to <code>std::tuple</code>. See also <code>std::tie</code>. <code>base::Tuple</code> is now an alias for <code>std::tuple</code>.</td> You could just say "Prefer over base::Tuple" and link to the bug. Like you did for ScopedVector.
The CQ bit was unchecked by danakj@chromium.org
(I un-CQ'd since I had a couple comments)
https://codereview.chromium.org/1892473003/diff/1/styleguide/c++/c++11.html File styleguide/c++/c++11.html (right): https://codereview.chromium.org/1892473003/diff/1/styleguide/c++/c++11.html#n... styleguide/c++/c++11.html:322: <td>Useful for fixed-size arrays. <a href="https://groups.google.com/a/chromium.org/d/topic/cxx/5iFNE8P5qT4/discussion">Discussion thread</a><br> Note that non-member <code>cbegin()</code> and <code>cend()</code> are not available until C++14.</td> On 2016/04/14 21:22:25, danakj wrote: > <br/>? > > Line breaking after a br makes sense in the HTML too I think. I'll reword this a bit to put the "Discussion thread" last, like it is on most other items, at which point the <br/> won't be needed. https://codereview.chromium.org/1892473003/diff/1/styleguide/c++/c++11.html#n... styleguide/c++/c++11.html:436: <td><a href="https://crbug.com/554987">Tracking bug</a> to plan moving from <code>base::Tuple</code> to <code>std::tuple</code>. See also <code>std::tie</code>. <code>base::Tuple</code> is now an alias for <code>std::tuple</code>.</td> On 2016/04/14 21:22:25, danakj wrote: > You could just say "Prefer over base::Tuple" and link to the bug. Like you did > for ScopedVector. Good idea.
The CQ bit was checked by pkasting@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from danakj@chromium.org, jbroman@chromium.org Link to the patchset: https://codereview.chromium.org/1892473003/#ps20001 (title: "Review comments")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1892473003/20001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1892473003/20001
Message was sent while issue was closed.
Description was changed from ========== C++11 doc cleanup: Changes with a visible effect: * Always comma-separate multiple items * Generally avoid <br>s except where listing code * Use the phrase "Google Style Guide", not "Google style guide", "C++ style guide", "Google Style Guide on XXX", etc. * declval item was in the wrong section, written poorly, and had broken links * Remove notes about MSVC2013 caveats * Fix formatting of code (use correct number of spaces, etc.) * Remove notes about FINAL and OVERRIDE macros, they're dead * Slight clarification to nullptr instructions to make it clear that replacing NULL in existing code is OK * Remove note forbidding general use of std::tuple since we explicitly allow general use of std::tuple * Change comments like "Reevaluate once MSVS2015 is available" to "Reevaluate now that MSVS2015 is available" * Add missing links to style guide for various banned items * Various other tiny wording fixes/clarifications Changes with no visible effect: * One space after a period * No line length limit (makes it easier to find an item to edit when scrolling through doc, matches how half of doc was already working, wrapping was very inconsistent otherwise) * Collapse together sequential <code> blocks with no intervening text BUG=none TEST=none ========== to ========== C++11 doc cleanup: Changes with a visible effect: * Always comma-separate multiple items * Generally avoid <br>s except where listing code * Use the phrase "Google Style Guide", not "Google style guide", "C++ style guide", "Google Style Guide on XXX", etc. * declval item was in the wrong section, written poorly, and had broken links * Remove notes about MSVC2013 caveats * Fix formatting of code (use correct number of spaces, etc.) * Remove notes about FINAL and OVERRIDE macros, they're dead * Slight clarification to nullptr instructions to make it clear that replacing NULL in existing code is OK * Remove note forbidding general use of std::tuple since we explicitly allow general use of std::tuple * Change comments like "Reevaluate once MSVS2015 is available" to "Reevaluate now that MSVS2015 is available" * Add missing links to style guide for various banned items * Various other tiny wording fixes/clarifications Changes with no visible effect: * One space after a period * No line length limit (makes it easier to find an item to edit when scrolling through doc, matches how half of doc was already working, wrapping was very inconsistent otherwise) * Collapse together sequential <code> blocks with no intervening text BUG=none TEST=none ==========
Message was sent while issue was closed.
Committed patchset #2 (id:20001)
Message was sent while issue was closed.
Description was changed from ========== C++11 doc cleanup: Changes with a visible effect: * Always comma-separate multiple items * Generally avoid <br>s except where listing code * Use the phrase "Google Style Guide", not "Google style guide", "C++ style guide", "Google Style Guide on XXX", etc. * declval item was in the wrong section, written poorly, and had broken links * Remove notes about MSVC2013 caveats * Fix formatting of code (use correct number of spaces, etc.) * Remove notes about FINAL and OVERRIDE macros, they're dead * Slight clarification to nullptr instructions to make it clear that replacing NULL in existing code is OK * Remove note forbidding general use of std::tuple since we explicitly allow general use of std::tuple * Change comments like "Reevaluate once MSVS2015 is available" to "Reevaluate now that MSVS2015 is available" * Add missing links to style guide for various banned items * Various other tiny wording fixes/clarifications Changes with no visible effect: * One space after a period * No line length limit (makes it easier to find an item to edit when scrolling through doc, matches how half of doc was already working, wrapping was very inconsistent otherwise) * Collapse together sequential <code> blocks with no intervening text BUG=none TEST=none ========== to ========== C++11 doc cleanup: Changes with a visible effect: * Always comma-separate multiple items * Generally avoid <br>s except where listing code * Use the phrase "Google Style Guide", not "Google style guide", "C++ style guide", "Google Style Guide on XXX", etc. * declval item was in the wrong section, written poorly, and had broken links * Remove notes about MSVC2013 caveats * Fix formatting of code (use correct number of spaces, etc.) * Remove notes about FINAL and OVERRIDE macros, they're dead * Slight clarification to nullptr instructions to make it clear that replacing NULL in existing code is OK * Remove note forbidding general use of std::tuple since we explicitly allow general use of std::tuple * Change comments like "Reevaluate once MSVS2015 is available" to "Reevaluate now that MSVS2015 is available" * Add missing links to style guide for various banned items * Various other tiny wording fixes/clarifications Changes with no visible effect: * One space after a period * No line length limit (makes it easier to find an item to edit when scrolling through doc, matches how half of doc was already working, wrapping was very inconsistent otherwise) * Collapse together sequential <code> blocks with no intervening text BUG=none TEST=none Committed: https://crrev.com/07c0959a98ec440e930b40445b0038af9fc1eca9 Cr-Commit-Position: refs/heads/master@{#387445} ==========
Message was sent while issue was closed.
Patchset 2 (id:??) landed as https://crrev.com/07c0959a98ec440e930b40445b0038af9fc1eca9 Cr-Commit-Position: refs/heads/master@{#387445} |
