Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

DM-29511: apply clang tidy cleanups #606

Merged
merged 1 commit into from Sep 13, 2021
Merged

DM-29511: apply clang tidy cleanups #606

merged 1 commit into from Sep 13, 2021

Conversation

mwittgen
Copy link
Contributor

@mwittgen mwittgen commented Sep 5, 2021

No description provided.

Copy link
Member

@TallJimbo TallJimbo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good. I'm happy with all of these fixes except (possibly):

  • I'd like to try to make the range-based for conversion use auto const & a little more often (see line comment).

  • I'm not sure of the value of [[nodiscard]] for us; I think it'll be visual noise for a lot of readers, and since we don't use return values for error propagation really at all, I don't think we're getting much. I can think of one big exception, but it's not relevant for afw: in geom and sphgeom, we have a lot of classes with both in-place and returning versions of many operations (e.g. flip operates in place, flipped returns a new geometry), and [[nodiscard]] on the returning versions would be very nice. I don't feel very strongly about this, though, so if it's a pain to remove just that commit I don't mind if it gets merged as well.

examples/rowColumnStats.cc Outdated Show resolved Hide resolved
src/geom/polygon/Polygon.cc Outdated Show resolved Hide resolved
apply clang-tidy fixes:
modernize-use-nullptr
modernize-use-emplace
modernize-deprecated-headers
modernize-use-using
modernize-modernize-loop-convert
modernize-use-equals-default
modernize-use-default-member-init
performance-implicit-conversion-in-loop
performance-faster-string-find
modernize-use-override
modernize-make-shared
@mwittgen mwittgen merged commit 184b6e4 into master Sep 13, 2021
@mwittgen mwittgen deleted the tickets/DM-29511 branch September 13, 2021 23:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants