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.

Sign up for GitHub

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

Jump to bottom

Update Detox to use react native 0.67 #3202

Merged
merged 28 commits into from
Mar 14, 2022
Merged

Update Detox to use react native 0.67 #3202

merged 28 commits into from
Mar 14, 2022

Conversation

jonathanmos
Copy link
Contributor

Description

Update Detox to use RN67. This fixes issue #3197

For features/enhancements:

  • I have added/updated the relevant references in the documentation files.

@jonathanmos jonathanmos changed the title Migrate to rn67 Migrate to react native 0.67.1 Feb 1, 2022
@jonathanmos jonathanmos changed the title Migrate to react native 0.67.1 Update Detox to use react native 0.67.1 Feb 1, 2022
asafkorem
asafkorem approved these changes Feb 1, 2022
View reviewed changes
Copy link
Contributor

@asafkorem asafkorem left a comment

Choose a reason for hiding this comment

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

LGTM

@jonathanmos jonathanmos changed the title Update Detox to use react native 0.67.1 Update Detox to use react native 0.67 Feb 9, 2022
asafkorem
asafkorem approved these changes Feb 13, 2022
View reviewed changes
Copy link
Contributor

@asafkorem asafkorem left a comment

Choose a reason for hiding this comment

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

Looks good on my end.

d4vidi
d4vidi reviewed Feb 13, 2022
View reviewed changes
README.md Show resolved Hide resolved
d4vidi
d4vidi reviewed Feb 13, 2022
View reviewed changes
scripts/ci.no.generate.sh Outdated Show resolved Hide resolved
d4vidi
d4vidi reviewed Feb 13, 2022
View reviewed changes
scripts/ci.no.generate.sh Outdated Show resolved Hide resolved
d4vidi
d4vidi requested changes Feb 13, 2022
View reviewed changes
Copy link
Collaborator

@d4vidi d4vidi left a comment

Choose a reason for hiding this comment

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

Mostly around my comment regarding ci.sh

d4vidi
d4vidi approved these changes Feb 22, 2022
View reviewed changes
Copy link
Collaborator

@d4vidi d4vidi left a comment

Choose a reason for hiding this comment

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

  • Please double check the jitpack.io thing
  • Please also attach links to green CI builds before merging (android & ios test jobs and android & ios example projects jobs) 🙏🏻

@jonathanmos
Copy link
Contributor Author

jonathanmos commented Feb 23, 2022 •
edited
Loading

Green builds for ios/android 64/66 test jobs -> https://jenkins-oss.wixpress.com/job/multi-detox-pr/4219/
Green build for ios 67 test job -> https://jenkins-oss.wixpress.com/view/all/job/detox-ios-67-branch/11/
Green build for android 67 test job -> https://jenkins-oss.wixpress.com/view/all/job/detox-android-67-x-branch/8/

d4vidi
d4vidi requested changes Feb 24, 2022
View reviewed changes
Copy link
Collaborator

@d4vidi d4vidi left a comment

Choose a reason for hiding this comment

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

Almost there... As the last mile, please be sure to run the demo-projects job using RN .67 as well. Also, please create an iOS demo-project job and wire it to our PR so it is executed regularly.

@d4vidi
Copy link
Collaborator

d4vidi commented Mar 1, 2022

@jonathanmos Touch-base; I see the current gaps are:

  • Reinstating the CI-script for the iOS example app (as in my previous comment)
  • Making the necessary adjustments in the buildkite scripts, in order to effectively upgrade the CI jobs to RN .67

@asafkorem
Copy link
Contributor

I removed detox-instruments phase from the iOS script, although I tried to make it work (I followed the instruments doc, didn't helped). I can investigate its issues separately, regardless of this PR.

@asafkorem asafkorem linked an issue Mar 3, 2022 that may be closed by this pull request
Upgrade to RN 0.67 #3197
Closed
4 tasks
@asafkorem
Copy link
Contributor

asafkorem commented Mar 3, 2022 •
edited
Loading

@jonathanmos Touch-base; I see the current gaps are:

Reinstating the CI-script for the iOS example app (as in my previous comment)
Making the necessary adjustments in the buildkite scripts, in order to effectively upgrade the CI jobs to RN .67

Done. Is there anything left that blocks us from merging this PR? @d4vidi @jonathanmos

@asafkorem asafkorem merged commit 78d786b into master Mar 14, 2022
@asafkorem asafkorem deleted the migrate-to-rn67 branch March 14, 2022 09:19
This was referenced Mar 14, 2022
Open
Closed
@d4vidi
Copy link
Collaborator

d4vidi commented Mar 15, 2022

!!!!!!

This was referenced Mar 26, 2022
Closed
Closed
@renovate renovate bot mentioned this pull request Apr 6, 2022
Closed
1 task
This was referenced Apr 26, 2022
Closed
Closed
@renovate renovate bot mentioned this pull request May 6, 2022
Closed
1 task
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@d4vidi d4vidi d4vidi requested changes

@asafkorem asafkorem asafkorem approved these changes

@noomorph noomorph Awaiting requested review from noomorph noomorph is a code owner

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Upgrade to RN 0.67
3 participants
@jonathanmos @d4vidi @asafkorem