-
Notifications
You must be signed in to change notification settings - Fork 741
[cxx20] Use defaulted comparison operators #4502
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
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #4502 +/- ##
==========================================
- Coverage 89.47% 89.47% -0.01%
==========================================
Files 242 243 +1
Lines 13880 13877 -3
==========================================
- Hits 12419 12416 -3
Misses 1461 1461 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
Doesn't build on MacOS, see discussion of IPAddress comparisons on my subnet refactor PR for context |
774dbf0 to
419e51d
Compare
Ah ha. Since you're already working on that, I'll just stay out of your way and remove the |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ubuntu builds and runs the tests without problems, but Windows 11 Pro fails the test: Utils/NormalizeMountTargetTest.mountTargetsNormalizeCorrectly/3, where GetParam() = ("../..//folder","/folder"). I am unsure if this is due to this PR, since the CI pipeline has been changed to use Windows Server 2025.
After some research I found that it may be to a missing rebase to origin/main, since in main it is fixed, and the normalize target mounts utils function is different due to your commit, so I will be approving this.
tobe2098
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
After some research I found that it may be to a missing rebase to origin/main, since in main it is fixed, and the normalize target mounts utils function is different due to your commit, so I will be approving this.
419e51d to
059b0c7
Compare
Thanks. I've rebased so hopefully everything passes CI now. |
|
It does pass now. Great job! |
Sploder12
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
This replaces a few manually-implemented comparison operators with the defaults. I guess these got missed in the big C++20 PR.