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.

Sign up for GitHub

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

Jump to bottom

Modernized code with clang-tidy [PCL-2731] #4629

Merged
merged 17 commits into from
Jun 19, 2022
Merged

Modernized code with clang-tidy [PCL-2731] #4629

merged 17 commits into from
Jun 19, 2022

Conversation

gnawme
Copy link
Contributor

@gnawme gnawme commented Feb 28, 2021

Modernization of code base using clang-tidy and an innocuous subset of its modernize checks:

  • modernize-avoid-bind
  • modernize-avoid-c-arrays
  • modernize-concat-nested-namespaces
  • modernize-deprecated-headers
  • modernize-deprecated-ios-base-aliases
  • modernize-loop-convert
  • modernize-make-shared
  • modernize-make-unique
  • modernize-pass-by-value
  • modernize-raw-string-literal
  • modernize-redundant-void-arg
  • modernize-replace-auto-ptr
  • modernize-replace-random-shuffle
  • modernize-return-braced-init-list
  • modernize-shrink-to-fit
  • modernize-unary-static-assert
  • modernize-use-auto
  • modernize-use-bool-literals
  • modernize-use-default-member-init
  • modernize-use-emplace
  • modernize-use-equals-default
  • modernize-use-equals-delete
  • modernize-use-nodiscard
  • modernize-use-noexcept
  • modernize-use-nullptr
  • modernize-use-override
  • modernize-use-trailing-return-type
  • modernize-use-transparent-functors
  • modernize-use-uncaught-exceptions
  • modernize-use-using

This PR splits off from #4560; per @kunaltyagi, it enables clang-tidy for the entire project, but omits the GitHub Workflow additions that would run clang-tidy checks in CI.

@gnawme gnawme changed the title Modernized code with clang-tidy Modernized code with clang-tidy [PCL-2731] Feb 28, 2021
@gnawme gnawme mentioned this pull request Mar 6, 2021
Merged
gnawme added 4 commits March 6, 2021 14:20
@gnawme
Bare-bones clang-tidy enabled [PCL-2731]
dc96f98
@gnawme
Ran clang-tidy modernization for all modules [PCL-2731]
5cfc686 
Signed-off-by: Norm Evangelista <norm.evangelista@gmail.com>
@gnawme
Fixed formatting issues flagged by CI [PCL-2731]
5203f5d 
Signed-off-by: Norm Evangelista <norm.evangelista@gmail.com>
@gnawme
Filled out WarningsAsErrors section in .clang-tidy [PCL-2731]
060d963 
Signed-off-by: Norm Evangelista <norm.evangelista@gmail.com>
@gnawme gnawme force-pushed the norm.evangelista/PCl-2731-clang-tidy-no-workflow branch from 248128c to 060d963 Compare March 6, 2021 22:21
@gnawme
Fixed formatting issue flagged by CI [PCL-2731]
6a35efa 
Signed-off-by: Norm Evangelista <norm.evangelista@gmail.com>
kunaltyagi
kunaltyagi reviewed Mar 10, 2021
View reviewed changes
Copy link
Member

@kunaltyagi kunaltyagi left a comment

Choose a reason for hiding this comment

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

This is a giant but pretty simple changeset. Reviewed 61/301 files

gnawme added 2 commits March 10, 2021 15:46
@gnawme
Placed '= default' on one line per review [PCL-2731]
6477cf7 
Signed-off-by: Norm Evangelista <norm.evangelista@gmail.com>
@gnawme
Onelined more '= default' declarations [PCL-2731]
ee00b9b 
Signed-off-by: Norm Evangelista <norm.evangelista@gmail.com>
kunaltyagi
kunaltyagi requested changes Mar 11, 2021
View reviewed changes
Copy link
Member

@kunaltyagi kunaltyagi left a comment

Choose a reason for hiding this comment

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

156/309 reviewed

gnawme added 2 commits March 10, 2021 22:19
@gnawme
One-lined more '= default' conversions [PCL-2731]
fd814ef 
Signed-off-by: Norm Evangelista <norm.evangelista@gmail.com>
@gnawme
Fixed a few EOL ;; instances [PCL-2731]
0aa88e8 
Signed-off-by: Norm Evangelista <norm.evangelista@gmail.com>
kunaltyagi
kunaltyagi requested changes Mar 11, 2021
View reviewed changes
Copy link
Member

@kunaltyagi kunaltyagi left a comment

Choose a reason for hiding this comment

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

There's still plenty left (split default).

You can catch them all using `git diff master | grep '^ += default'

gnawme added 2 commits March 11, 2021 10:57
@gnawme
Caught some escapes from recent rebase [PCL-2731]
d4f614d 
Signed-off-by: Norm Evangelista <norm.evangelista@gmail.com>
@gnawme
Fixed one overlooked conversion to "using" [PCL-2731]
948f06e 
Signed-off-by: Norm Evangelista <norm.evangelista@gmail.com>
@gnawme
Copy link
Contributor Author

gnawme commented Mar 12, 2021

There's still plenty left (split default).

You can catch them all using `git diff master | grep '^ += default'

I ran that regex in my IDE originally. Running this on the command line didn't display any others.

kunaltyagi
kunaltyagi requested changes Mar 12, 2021
View reviewed changes
Copy link
Member

@kunaltyagi kunaltyagi left a comment

Choose a reason for hiding this comment

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

268/317 done

Almost all other files have similar issues (space or split default)

gnawme added 4 commits March 12, 2021 08:18
@gnawme
Fixed yet more split '= default' instances [PCL-2731]
b3d834d 
Signed-off-by: Norm Evangelista <norm.evangelista@gmail.com>
@gnawme
Reverted clang-tidy changes for C header [PCL-2731]
4bc502b 
Signed-off-by: Norm Evangelista <norm.evangelista@gmail.com>
@gnawme
Fixed yet more split '= default' [PCL-2731]
0255b69 
Signed-off-by: Norm Evangelista <norm.evangelista@gmail.com>
@gnawme
Caught another split '= default' [PCL-2731]
a92ee8e 
Signed-off-by: Norm Evangelista <norm.evangelista@gmail.com>
kunaltyagi
kunaltyagi requested changes Mar 13, 2021
View reviewed changes
Copy link
Member

@kunaltyagi kunaltyagi left a comment

Choose a reason for hiding this comment

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

310/316 files reviewed

@gnawme
Fixed issues flagged by review [PCL-2731]
d2543d5 
Signed-off-by: Norm Evangelista <norm.evangelista@gmail.com>
kunaltyagi
kunaltyagi previously approved these changes Mar 13, 2021
View reviewed changes
Copy link
Member

@kunaltyagi kunaltyagi left a comment

Choose a reason for hiding this comment

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

LGTM

Thanks for bearing with us 😸

@gnawme
Copy link
Contributor Author

gnawme commented Mar 14, 2021

LGTM

Thanks for bearing with us

Happy to help.

@ueqri ueqri mentioned this pull request Jun 5, 2021
Open
@mvieth mvieth added the changelog: enhancement Meta-information for changelog generation label Jun 19, 2022
mvieth
mvieth approved these changes Jun 19, 2022
View reviewed changes
Copy link
Member

@mvieth mvieth left a comment

Choose a reason for hiding this comment

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

Thank you, and sorry for the delay

@mvieth mvieth merged commit 3a40875 into PointCloudLibrary:master Jun 19, 2022
@mvieth mvieth mentioned this pull request Jun 24, 2022
Closed
30 tasks
@gnawme gnawme mentioned this pull request Jun 25, 2022
Closed
mvieth pushed a commit that referenced this pull request Jul 3, 2022
@gnawme
Fix clang-tidy issues arisen since #4629 (#5310)
02c9c74 
* Ran clang-tidy-14 on codebase [PCL-5304]

* Corrected issues flagged by clang-tidy in CI [PCL-5304]

* Fixed out-of-order decl [PCL-5304]

* Fixed using declaration [PCL-5304]
gnawme added a commit to gnawme/pcl that referenced this pull request Jul 4, 2022
@gnawme
Fix clang-tidy issues arisen since PointCloudLibrary#4629 (PointCloud…
928d9ad 
…Library#5310)

* Ran clang-tidy-14 on codebase [PCL-5304]

* Corrected issues flagged by clang-tidy in CI [PCL-5304]

* Fixed out-of-order decl [PCL-5304]

* Fixed using declaration [PCL-5304]
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mvieth mvieth mvieth approved these changes

@kunaltyagi kunaltyagi kunaltyagi left review comments

Assignees
No one assigned
Labels
changelog: enhancement Meta-information for changelog generation
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@gnawme @kunaltyagi @mvieth