Fix Doxygen warnings in column header files #10963
Conversation
karthikeyann
added
3 - Ready for Review
Codecov Report
@@ Coverage Diff @@
## branch-22.08 #10963 +/- ##
===============================================
Coverage ? 86.33%
===============================================
Files ? 144
Lines ? 22706
Branches ? 0
===============================================
Hits ? 19604
Misses ? 3102
Partials ? 0 Continue to review full report at Codecov.
|
Co-authored-by: Vyas Ramasubramani <[email protected]>
There was a problem hiding this comment.
@karthikeyann Sorry for the delay in review. I started this review when you requested it and realized I hadn't submitted the review comments while cleaning my email inbox. Please find some suggestions attached. I am happy with any way you want to resolve the suggestions, so I am approving. I won't need to take another round of review unless you have questions. Thanks for your efforts on this!
| */ | ||
| template <typename T> | ||
| static constexpr bool has_element_accessor() | ||
| { | ||
| return has_element_accessor_impl<column_device_view, T>::value; | ||
| } | ||
|
|
||
| /// Counting iterator | ||
| using count_it = thrust::counting_iterator<size_type>; |
There was a problem hiding this comment.
Hmm, this is a little awkward. We are defining a public member type count_it but it seems like this is only used for shorthand inside this class definition. The value typename column_device_view::count_it is never used. I wonder if this should be refactored in a separate PR.
It's normal to define iterator type members like const_iterator and iterator (for the mutable column device view) for containers, but count_it doesn't fit that category.
There was a problem hiding this comment.
Likely, this type of fixes needs a global cleanup.
Co-authored-by: Bradley Dice <[email protected]>
|
@gpucibot merge |
Fixes parts of #9373
added missing documentation to fix doxygen warnings in column, column_view, column_device_view, column factories headers
fixes 155 warnings.