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

Enable verse block #2185

Merged
merged 39 commits into from
May 18, 2020
Merged

Enable verse block #2185

merged 39 commits into from
May 18, 2020

Conversation

guarani
Copy link
Contributor

@guarani guarani commented Apr 24, 2020

Addresses #1886

To test: See WordPress/gutenberg#21883

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.

@guarani guarani changed the title Rnmobile/enable verse block Enable verse block Apr 24, 2020
@guarani
Copy link
Contributor Author

guarani commented Apr 28, 2020

I'd love to hear if there's a better workflow to create downloadable builds. For this PR, I ran these commands and committed the results:

  • yarn bundle:ios
  • yarn bundle:android

This allows the associated PRs, wordpress-mobile/WordPress-iOS#14004 and wordpress-mobile/WordPress-Android#11763 to find a packaged App.js file for inclusion in the build.
The major disadvantage here is that these bundle commands add ~100 files to the PR and this compilactes the task of reviewing the code.

@guarani guarani marked this pull request as ready for review April 28, 2020 23:45
@guarani guarani requested a review from etoledom April 28, 2020 23:47
@mchowning
Copy link
Contributor

I'd love to hear if there's a better workflow to create downloadable builds. For this PR, I ran these commands and committed the results:

yarn bundle:ios
yarn bundle:android

At this time I think that's necessary for iOS. Android doesn't need the bundles because they're handled by jitpack, but it's probably better to go ahead and bundle both to keep them in sync. I agree that it does make it very painful to review PRs. I definitely think we should try to work toward not including the bundles in the repo like this eventually.

@guarani guarani mentioned this pull request May 4, 2020
6 tasks
@guarani guarani requested a review from ceyhun May 11, 2020 13:37
@SergioEstevao SergioEstevao added this to the 1.28 milestone May 11, 2020
@SergioEstevao SergioEstevao modified the milestones: 1.28, 1.29 May 13, 2020
@SergioEstevao SergioEstevao self-requested a review May 18, 2020 20:15
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.

All working well, extra kudos for adding the unit tests!

@guarani guarani merged commit 399ef0b into develop May 18, 2020
@guarani guarani deleted the rnmobile/enable-verse-block branch May 18, 2020 22:52
@guarani
Copy link
Contributor Author

guarani commented May 18, 2020

All working well, extra kudos for adding the unit tests!

Thanks @SergioEstevao, merged now! 🎆

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