Skip to content

Conversation

@Tug
Copy link
Contributor

@Tug Tug commented Apr 21, 2020

Description

The goal of this PR is to target the gutenberg monorepo branch. With this change gutenberg will receive more responsibility and gutenberg-mobile becomes a "customized experience" of the core RN editor. The idea is to have tests at both places. On gutenberg, integrated as part of the default CI and here as part of our usual contributions and releases.

Gutenberg PR: WordPress/gutenberg#21332
WordPress-Android PR: wordpress-mobile/WordPress-Android#11956
WordPress-iOS PR: wordpress-mobile/WordPress-iOS#14188

Testing instructions

Check that all the following commands are working correctly. When running the "enhanced" gutenberg mobile you should see "Welcome to gutenberg for WP apps" as the post title while it should just display "Welcome to gutenberg" for the core version.

  • npm install
  • npm start
  • npm run start:reset
  • npm run core android
  • npm run core ios
  • npm run genstrings
  • npm run bundle
  • npm run test
  • npm run core test
  • npm run test:e2e:android:local
  • npm run test:e2e:ios:local
  • npm run core test:e2e:android:local
  • npm run core test:e2e:ios:local
  • npm run wpandroid
  • Running the WPAndroid app in debug mode
  • Running the WPAndroid app in prod mode
  • Running the WPiOS app in debug mode
  • Running the WPiOS app in prod mode
  • CI tests running green in this PR
  • CI tests running green in the gutenberg PR

Code review

The "Hide deleted files" option on github should help with the review:

PR submission checklist:

  • I have considered adding unit tests where possible.
  • I have considered if this change warrants user-facing release notes and have added them to RELEASE-NOTES.txt if necessary.

@Tug Tug self-assigned this Apr 21, 2020
@Tug Tug force-pushed the try/after-monorepo branch from 884ace5 to cfbe041 Compare April 28, 2020 16:49
@Tug Tug requested review from cameronvoell, ceyhun and hypest April 28, 2020 16:50
@Tug Tug added this to the 1.29 milestone May 15, 2020
@hypest
Copy link
Contributor

hypest commented Jun 19, 2020

Pushed a patch based solution for the patching issue @cameronvoell reported here. Note that the solution won't probably be compatible with Windows as it's based on patch. It's a stopgap solution and we should revise with something cleaner.

In case a git apply plus pipes exit codes work better on Windows, here's an alternative method we might be able to use:

"patch-react-native": "echo "Will test if the RN patch has been applied..."; git apply --check --reverse --ignore-whitespace patches/react-native+0.61.5.patch || (git apply --ignore-whitespace patches/react-native+0.61.5.patch && echo "Applied RN patch!")",

It's checking to reverse the patch first, and if fails it means that the patch is not applied so, going ahead with applying it.

@ceyhun
Copy link
Contributor

ceyhun commented Jun 22, 2020

@hypest The patch is printing Skipping patch. on CircleCI although it's a fresh npm install and it should be applied. I think it's using the wrong -p option. It should be -p1 instead of -p0.
Confirmed this also running locally, the patch is not applied right now.

Edit: Pushed the fix

@ceyhun ceyhun force-pushed the try/after-monorepo branch 2 times, most recently from 3fac01a to ef092c1 Compare June 22, 2020 16:24
@cameronvoell cameronvoell changed the base branch from develop-monorepo to develop June 23, 2020 04:59
@peril-wordpress-mobile
Copy link

peril-wordpress-mobile bot commented Jun 23, 2020

Wanna run full suite of Android and iOS UI tests? Click here and 'Approve' CI job!

@cameronvoell cameronvoell merged commit cf0a4f9 into develop Jun 23, 2020
@cameronvoell cameronvoell deleted the try/after-monorepo branch June 23, 2020 18:02
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.

8 participants