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 react-native-aztec-old-submodule #1861

Merged
merged 4 commits into from
Feb 18, 2020

Conversation

ceyhun
Copy link
Contributor

@ceyhun ceyhun commented Feb 6, 2020

react-native-aztec was moved into this repo a while ago and it's submodule was removed: #465
But this submodule was re-introduced again because of JitPack problems: #486
Seems like JitPack is not an issue anymore as the CI is green in this PR so we can remove this submodule now.

@ceyhun ceyhun changed the title Remove react-native-aztec-old-submodule [WIP] Remove react-native-aztec-old-submodule Feb 6, 2020
@ceyhun ceyhun marked this pull request as ready for review February 6, 2020 19:17
@ceyhun ceyhun changed the title [WIP] Remove react-native-aztec-old-submodule Remove react-native-aztec-old-submodule Feb 6, 2020
@ceyhun ceyhun requested review from hypest and mchowning and removed request for mchowning February 7, 2020 10:07
@Tug
Copy link
Contributor

Tug commented Feb 12, 2020

Changes looks good to me. Could you maybe open a PR on WPAndroid that targets this branch to make sure ewverything is ok now?

@ceyhun ceyhun force-pushed the try/remove-react-native-aztec-old-submodule branch from fc7493e to c972d0d Compare February 12, 2020 20:41
@ceyhun
Copy link
Contributor Author

ceyhun commented Feb 13, 2020

@hypest
Copy link
Contributor

hypest commented Feb 13, 2020

Looking at the JitPack log:

jest-haste-map: Haste module naming collision: react-native-aztec
The following files share their name; please adjust your hasteImpl:

  • /react-native-aztec-old-submodule/package.json
  • /react-native-aztec/package.json

Looks like the "old-submodule" is still there. Not fully sure yet if this is essentially the same error that we got back in the day and that's why we decided to keep the old-submodule around 🤔 .

@hypest
Copy link
Contributor

hypest commented Feb 13, 2020

Or, perhaps we have to add a step in JitPack's config to rm -Rf the submodule folder which is probably checked out from the initial clone (before JitPack then switches the to branch under test). Does that make sense?

@ceyhun
Copy link
Contributor Author

ceyhun commented Feb 13, 2020

Not fully sure yet if this is essentially the same error that we got back in the day and that's why we decided to keep the old-submodule around 🤔 .

I think it was a different error before jitpack/jitpack.io#3707 mentioned here #486.

Or, perhaps we have to add a step in JitPack's config to rm -Rf the submodule folder which is probably checked out from the initial clone (before JitPack then switches the to branch under test). Does that make sense?

Let me try that 👍

@ceyhun
Copy link
Contributor Author

ceyhun commented Feb 13, 2020

It worked perfectly 🎉 Android build is now green as well: wordpress-mobile/WordPress-Android#11289

@ceyhun ceyhun requested a review from hypest February 13, 2020 15:19
Copy link
Contributor

@SergioEstevao SergioEstevao left a comment

Choose a reason for hiding this comment

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

:shipit:

@ceyhun ceyhun merged commit 2f627a9 into develop Feb 18, 2020
@ceyhun ceyhun deleted the try/remove-react-native-aztec-old-submodule branch February 18, 2020 10:59
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.

4 participants