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

fix: Replace local headers with quotes (") #1539

Merged
merged 2 commits into from
May 3, 2023

Conversation

mrousavy
Copy link
Contributor

@mrousavy mrousavy commented May 2, 2023

Local headers should be imported with " instead of <.

Screenshot 2023-05-02 at 18 32 24

I replaced all the headers by running:

cd package/cpp
sed -i "" -r "s/#(import|include) <((RN|Jsi|Sk)[a-zA-Z0-9.]*)>/#\1 \"\2\"/g" ./**/*.h

And then I manually replaced two occasions (DisplayLink.h and something else)

@chrfalch chrfalch self-requested a review May 2, 2023 16:22
@chrfalch
Copy link
Contributor

chrfalch commented May 2, 2023

This is also a continuation of #1113 - which did the same but didn't do it for all includes. So this one fixes that one :) :)

Copy link
Contributor

@chrfalch chrfalch left a comment

Choose a reason for hiding this comment

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

Nice - I've built this on both Android and iOS and it works (as expected) well.

@wcandillon wcandillon closed this May 3, 2023
@wcandillon wcandillon reopened this May 3, 2023
@wcandillon
Copy link
Contributor

looks great, let me run the ci on it quickly and we will release it asap

@wcandillon wcandillon changed the base branch from main to integration2 May 3, 2023 06:44
@wcandillon wcandillon merged commit 0624c83 into Shopify:integration2 May 3, 2023
wcandillon added a commit that referenced this pull request May 3, 2023
* fix: Replace local headers with quotes (`"`) (#1539)

* fix: Replace local headers with quotes (`"`)

* fix: Also replace `Jsi*` headers

* fix ci 💚

---------

Co-authored-by: Marc Rousavy <[email protected]>
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.

3 participants