[PATCH] D57353: [clang-tidy] Add the abseil-duration-double-conversion check

MyDeveloperDay via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Thu Jan 31 02:40:47 PST 2019


MyDeveloperDay added inline comments.


================
Comment at: docs/clang-tidy/checks/abseil-duration-double-conversion.rst:20
+
+
+  // Original - Conversion to integer and back again
----------------
hwright wrote:
> MyDeveloperDay wrote:
> > hwright wrote:
> > > Eugene.Zelenko wrote:
> > > > Unnecessary empty line.
> > > This is consistent with other documentation in this directory, such as `abseil-faster-strsplit-delimiter.rst`.
> > In your example `abseil-faster-strsplit-delimiter.rst` , The double blank line in the html doesn't give much delineation between the before and after code and the next example.
> > 
> > {F7867869}
> > 
> > There probably isn't a convention per say (which is a shame), across the docs we do a mixture of different styles 
> > 
> > https://clang.llvm.org/extra/clang-tidy/checks/readability-braces-around-statements.html
> > https://clang.llvm.org/extra/clang-tidy/checks/android-cloexec-accept.html
> > https://clang.llvm.org/extra/clang-tidy/checks/google-objc-function-naming.html
> > 
> > But there is a desire by some of the regular clang-tidy reviewers to make the documentation consistent
> > 
> > It may not be ideal but the "Before/After" style, that is used in `modernize-use-emplace`, `modernize-use-using`,`readability-braces-around-statements`,`readability-identifier-naming` and `readability-redundant-function-ptr-dereference` does help a little.
> > 
> > I'm not saying looks better, but I've added a couple of examples of formatting the strsplit example for comparison, feel free to ignore.
> > 
> > {F7867960}
> > 
> > {F7868028}
> > 
> > 
> I like those examples!
> 
> Would it be reasonable to update all of the `abseil-duration-*` documentation in a separate pass, after this change is submitted?
If it means the documentation gets better in the long run, I'm fine with that, its mainly a Nit on my part.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D57353/new/

https://reviews.llvm.org/D57353





More information about the cfe-commits mailing list