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

Update fork to 5.0.2 #7

Closed
wants to merge 25 commits into from

Conversation

SergioEstevao
Copy link

@SergioEstevao SergioEstevao commented Apr 29, 2020

Update fork to version 5.0.2 of the original repo.

jhalvorson and others added 24 commits August 17, 2019 09:44
…on super from RCTVideo.

If the super class is not actually observing the key, the app will crash. Checking to see if
the super class responds to this selector doesn't solve this issue.

react-native-video github issue: TheWidlarzGroup#1515

Discussion about this particular problem: https://stackoverflow.com/questions/6574714/whats-wrong-with-this-observevalueforkeypathofobjectchangecontext-implement
Updated README.md to include instructions for React Native 0.60 and a…
…consistencyException

Removing the call to observeValueForKeyPath:ofObject:change:context: …
This reverts commit f7383da.

Reverting as it appears that code changes will be needed to the
ReactExoplayerView to support the upgrade of the ExoPlayer lib.
@SergioEstevao SergioEstevao requested a review from hypest April 29, 2020 15:41
@SergioEstevao SergioEstevao added the enhancement New feature or request label Apr 29, 2020
@SergioEstevao SergioEstevao requested a review from mchowning April 29, 2020 15:41
# Conflicts:
#	android-exoplayer/build.gradle
#	android/build.gradle
@hypest
Copy link

hypest commented Apr 30, 2020

👋 Sérgio, let's open a gutenberg-mobile PR as well, to test this and only merge when everything looks ok. Thanks!

@hypest
Copy link

hypest commented Apr 30, 2020

Is seems that many commits are already on the fork-for-gutenberg-mobile branch. Any info on why this PR/branch adds them again?

@SergioEstevao
Copy link
Author

SergioEstevao commented Apr 30, 2020 via email

@hypest
Copy link

hypest commented Apr 30, 2020

I did a rebase of 5.0.2 tag to this branch...could that be the reason?

Hmm, most probably, yeah. Thing is, even though the diff itself is fairly trivial and easy to review, it's not clear what commits we bring in from master (which tracks upstream) and messes up the history, possibly making debugging issues a bit harder. Can we do a branch merge instead of a rebase instead? Were there non-trivial conflicts perhaps, if you tried it?

@SergioEstevao
Copy link
Author

Closed in favour of this PR: #8

@hypest can you give a look to the PR above?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants