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

Remove deprecated std::iterator inheritance #529

Merged
merged 4 commits into from
Sep 28, 2023

Conversation

mjcarroll
Copy link
Contributor

🦟 Bug fix

Fixes #505

Summary

Removes the use of the deprecated std::iterator template and replaces it with using the correct set of using statements.

I also added a gzEnum variadic template at clang's suggestion. This should probably be preferred over the macro where relevant.

Checklist

  • Signed all commits for DCO
  • Added tests
  • Updated documentation (as needed)
  • Updated migration guide (as needed)
  • Consider updating Python bindings (if the library has them)
  • codecheck passed (See contributing)
  • All tests passed (See test coverage)
  • While waiting for a review on your PR, please help review another open pull request to support the maintainers

Note to maintainers: Remember to use Squash-Merge and edit the commit message to match the pull request summary while retaining Signed-off-by messages.

@mjcarroll mjcarroll requested a review from marcoag as a code owner August 14, 2023 22:02
@mjcarroll mjcarroll requested a review from scpeters September 26, 2023 20:35
@iche033 iche033 mentioned this pull request Sep 26, 2023
@scpeters
Copy link
Member

/github/workspace/src/EnumIface_TEST.cc:36:32: error: 'gzEnum' is not a member of 'gz::common'; did you mean 'gzEnum'?
     36 | auto myTypeIface = gz::common::gzEnum(
        |                                ^~~~~~
  In file included from /github/workspace/src/EnumIface_TEST.cc:20:
  /github/workspace/include/gz/common/EnumIface.hh:240:43: note: 'gzEnum' declared here
    240 | static constexpr gz::common::EnumIface<T> gzEnum(
        |                                           ^~~~~~

@codecov
Copy link

codecov bot commented Sep 27, 2023

Codecov Report

Merging #529 (c35a188) into main (12872b3) will increase coverage by 0.00%.
The diff coverage is 100.00%.

❗ Current head c35a188 differs from pull request most recent head 4d78133. Consider uploading reports for the commit 4d78133 to get more accurate results

@@           Coverage Diff           @@
##             main     #529   +/-   ##
=======================================
  Coverage   83.21%   83.22%           
=======================================
  Files          82       82           
  Lines        9880     9883    +3     
=======================================
+ Hits         8222     8225    +3     
  Misses       1658     1658           
Files Coverage Δ
include/gz/common/EnumIface.hh 100.00% <100.00%> (ø)

@mjcarroll
Copy link
Contributor Author

Windows failure was a flake here.

@mjcarroll mjcarroll merged commit e5465f1 into main Sep 28, 2023
10 checks passed
@mjcarroll mjcarroll deleted the mjcarroll/rm_deprecated_iterator branch September 28, 2023 22:19
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.

2 participants