Upgrade react-native-video and drop jetifier on Android #38426
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This was referenced Feb 1, 2022
|
Size Change: +5.71 kB (+1%) Total Size: 1.15 MB
ℹ️ View Unchanged
|
|
I'm going to keep this a draft until we update the react-native-video dependency to not reference a git commit hash, which we can do once wordpress-mobile/react-native-video#15 is merged). |
mchowning
force-pushed
the
rnmobile/drop_jetifier
branch
from
b234d72 to
b094190
Compare
mchowning
force-pushed
the
rnmobile/drop_jetifier
branch
from
b094190 to
9097bd6
Compare
hypest
reviewed
Feb 2, 2022
package-lock.json
Outdated
| "version": "https://raw.githubusercontent.com/wordpress-mobile/react-native-video/5.0.2-wp-2/react-native-video-5.0.2-wp-2.tgz", | ||
| "integrity": "sha512-JcZKEFDbje8MNEd1sEO5eQDrF7/BUaYMskI2G7rTXd0EwOPGx9rHOdx0uAmmew+NvkMNPmh+H6uKsgiaS891dA==", | ||
| "version": "git+https://github.com/wordpress-mobile/react-native-video.git#31ebacf5584219d4b3888b95673c35243fd30d2e", | ||
| "from": "git+https://github.com/wordpress-mobile/react-native-video.git#31ebacf5584219d4b3888b95673c35243fd30d2e", |
There was a problem hiding this comment.
Not sure, will there be a problem that the integrity field is now missing? 🤔
There was a problem hiding this comment.
I believe that field will come back when I switch from a github commit to a tar.gz file since it sounds like when using a git commit just the commit hash itself is used to insure integrity.
With that said, the static lint checks are failing because of the package-lock file, and I haven't been able to figure out what is going on with that yet.
1. Copied package-lock.json file from trunk 2. Removed react-native-video dep from packages/react-native-editor/package.json 3. Ran `npm install` 4. Restored react-native-video dep 5. Ran `npm install`
|
Marking this as ready for review, but we will still need to update the react-native-video ref to point to the tag and not the branch once wordpress-mobile/react-native-video#15 is merged and we create the |
cbravobernal
added
the
Mobile App - i.e. Android or iOS
This was referenced Mar 1, 2022
This was referenced Mar 15, 2022
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
.tgzfile before merging this.Related gutenberg-mobile PR: wordpress-mobile/react-native-video@5c6697a
Description
This PR updates react-native-video to allow us to drop jetifier on Android.
Testing Instructions
Using the demo app, play the video from the sample content. On Android this should route to an external application (probably the web browser) and either play the video or offer to download it. On iOS though, the demo video should open and play within the demo app itself.
Screenshots
Types of changes
Checklist:
*.native.jsfiles for terms that need renaming or removal).