-
Notifications
You must be signed in to change notification settings - Fork 269
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
refactor: use concepts even more #5538
refactor: use concepts even more #5538
Conversation
e51c14c
to
a5acaa8
Compare
a few build errors on this one still :( |
b6970e0
to
a3c18af
Compare
a3c18af
to
bb6af8c
Compare
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.
This was a pretty big PR, but thankfully wasn't doing anything weird. It was easy to see why the changes were being made blah<T>::type
to blah_t<T>
and blah<T>::value
to blah_v<T>
mostly, along with changing over from enable_if
to requires
syntax
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.
It's a lot to go over but it compiles and loads, and only tests that're failing are clang whining about unrelated things again: https://github.com/cataclysmbnteam/Cataclysm-BN/actions/runs/11241032936/job/31251730123?pr=5538
Co-authored-by: OrenAudeles <[email protected]>
b535e4c
to
7eab273
Compare
Checklist
Required
main
so it won't cause conflict when updatingmain
branch later.Purpose of change
followup to #5537: apply https://clang.llvm.org/extra/clang-tidy/checks/modernize/use-constraints.html
Describe the solution
ran:
btw it took 370s lol
Testing
should (probably) build