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

feat(exoplayerview): Migrate ExoPlayerView to kotlin #4038

Merged
merged 4 commits into from
Oct 11, 2024
Merged

feat(exoplayerview): Migrate ExoPlayerView to kotlin #4038

merged 4 commits into from
Oct 11, 2024

Conversation

l2hyunwoo
Copy link
Contributor

Summary

Migrate ExoPlayerView to Kotlin

Motivation

Migrate the codebase from Java to Kotlin

Changes

Test plan

@seyedmostafahasani
Copy link
Collaborator

@l2hyunwoo I have 3 small comments.

@l2hyunwoo
Copy link
Contributor Author

@seyedmostafahasani applied your review 👍🏻 thanks

@moskalakamil
Copy link
Member

Hi @l2hyunwoo, could you please resolve conflicts?

@seyedmostafahasani
Copy link
Collaborator

@l2hyunwoo
Would you please resolve conflicts again :)?

@l2hyunwoo l2hyunwoo force-pushed the feature/migrate-exoplayerview-to-kotlin branch from b99fe2a to ca9653a Compare October 2, 2024 08:14
@l2hyunwoo
Copy link
Contributor Author

@moskalakamil @seyedmostafahasani I resolved it!

@YangJonghun
Copy link
Collaborator

@freeboub @KrzysztofMoch
Could we merge this PR?

@freeboub
Copy link
Collaborator

freeboub commented Oct 10, 2024 •
edited
Loading

@YangJonghun I just merge a PR which ads 2 small functions and a visibility change
@l2hyunwoo Can you integrate then and I am ok to merge this PR

Sorry for the review delay ....

The new functions:

    public void showAds() {
        adOverlayFrameLayout.setVisibility(View.GONE);
    }
    public void hideAds() {
        adOverlayFrameLayout.setVisibility(View.VISIBLE);
    }

https://github.com/TheWidlarzGroup/react-native-video/pull/4185/files#diff-260c33b2f3f6db3a66072d98f3be84f76badc2648c4b49ade349a4b1914c08db

@freeboub
Copy link
Collaborator

this PR will also conflict ...

@l2hyunwoo
Rename .java to .kt
5fefd22 
Migrate ExoPlayerView to Kotlin

Lint fix

Fix build (by isPlaying)

Apply lint

Apply code review

Apply code review

Apply code review
@l2hyunwoo
Lint apply
4bfa6d4
@l2hyunwoo
Fix build issue
3df8cf1
@l2hyunwoo
Update functions
77a87c8
@l2hyunwoo l2hyunwoo force-pushed the feature/migrate-exoplayerview-to-kotlin branch from be2d224 to 77a87c8 Compare October 11, 2024 06:54
@l2hyunwoo
Copy link
Contributor Author

@freeboub i resolved conflicts!

@freeboub
Copy link
Collaborator

Let's go with that !

@freeboub freeboub merged commit 78f4f04 into TheWidlarzGroup:master Oct 11, 2024
3 checks passed
@l2hyunwoo l2hyunwoo deleted the feature/migrate-exoplayerview-to-kotlin branch October 11, 2024 07:21
KrzysztofMoch pushed a commit that referenced this pull request Oct 11, 2024
@l2hyunwoo @KrzysztofMoch
feat(exoplayerview): Migrate ExoPlayerView to kotlin (#4038)
9056054
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@seyedmostafahasani seyedmostafahasani seyedmostafahasani approved these changes

@YangJonghun YangJonghun YangJonghun left review comments

@freeboub freeboub freeboub left review comments

@KrzysztofMoch KrzysztofMoch KrzysztofMoch approved these changes

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

Successfully merging this pull request may close these issues.
6 participants
@l2hyunwoo @seyedmostafahasani @moskalakamil @YangJonghun @freeboub @KrzysztofMoch