-
-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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
Convert double-quoted includes to angle includes #6537
Conversation
Plz fix import orders for CI 🙏 |
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.
All good, I mostly addressed some inconsistencies but feel free to address them as you like.
Personally I'm not a fan of separating includes with newlines but since we don't have any tools for that I guess it's fine.
packages/react-native-reanimated/Common/cpp/reanimated/Fabric/ReanimatedCommitShadowNode.h
Show resolved
Hide resolved
packages/react-native-reanimated/Common/cpp/reanimated/NativeModules/NativeReanimatedModule.cpp
Outdated
Show resolved
Hide resolved
packages/react-native-reanimated/Common/cpp/reanimated/NativeModules/NativeReanimatedModule.cpp
Outdated
Show resolved
Hide resolved
packages/react-native-reanimated/Common/cpp/reanimated/NativeModules/NativeReanimatedModule.h
Outdated
Show resolved
Hide resolved
packages/react-native-reanimated/Common/cpp/reanimated/NativeModules/NativeReanimatedModule.cpp
Outdated
Show resolved
Hide resolved
packages/react-native-reanimated/android/src/main/cpp/reanimated/NativeProxy.h
Outdated
Show resolved
Hide resolved
packages/react-native-reanimated/android/src/main/cpp/worklets/CMakeLists.txt
Show resolved
Hide resolved
…odules/NativeReanimatedModule.cpp Co-authored-by: Tomasz Żelawski <40713406+tjzel@users.noreply.github.com>
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.
👍
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.
## Summary PR adding back headers removed in #6537. For some reason, locally they are not needed, probably to some different setup or having the libraries set globally somewhere, but it caused failures in Expensify: https://github.com/Expensify/App/actions/runs/11857735322/job/33046828856. They should be included since methods from them are used. ## Test plan Not really, just run CIs probably.
## Summary PR adding back headers removed in #6537. For some reason, locally they are not needed, probably to some different setup or having the libraries set globally somewhere, but it caused failures in Expensify: https://github.com/Expensify/App/actions/runs/11857735322/job/33046828856. They should be included since methods from them are used. ## Test plan Not really, just run CIs probably.
Summary
This PR:
#include "someheader.h
to#include <reanimated/path/to/someheader.h>
CMakeLists.txt
andRNReanimated.podspec
) to change header pathsCommon/cpp/reanimated
orCommon/cpp/worklets
, as well as one forapple/reanimated
, so that header paths are correct (otherwise CocoaPods flattens nested directories)yarn lint:{android,apple,common}
)Next PRs:
#import <RNReanimated/*.h>
to#import <reanimated/**/*.h>
inapple/
directoryreanimated
C++ namespace toswmansion::reanimated
andswmansion::worklets
Test plan