Skip to content

Add loaded event to the player#2441

Merged
joeyparrish merged 1 commit intoshaka-project:masterfrom
avelad:loaded-event
Mar 4, 2020
Merged

Add loaded event to the player#2441
joeyparrish merged 1 commit intoshaka-project:masterfrom
avelad:loaded-event

Conversation

@avelad
Copy link
Member

@avelad avelad commented Mar 4, 2020

Add a new event in the player that indicate that the load is complete.

This is necessary for SS ADS because it does the player.load process.

Copy link
Member

@joeyparrish joeyparrish left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks reasonable. Can you give us a little more detail on why SS ads needs this event?

@shaka-bot
Copy link
Collaborator

All tests passed!

@avelad
Copy link
Member Author

avelad commented Mar 4, 2020

The SS manage by it self the load in https://github.com/google/shaka-player/blob/master/lib/ads/server_side_ad_manager.js#L293

In my app logic I need get the resolved promise for execute some actions. Due that now there are not promise, I need a event for execute the actions

@joeyparrish
Copy link
Member

I'll go ahead and merge this, but is there any need for a corresponding Promise from the ad manager?

@joeyparrish joeyparrish merged commit 57ce383 into shaka-project:master Mar 4, 2020
@joeyparrish joeyparrish added this to the v2.6 milestone Mar 4, 2020
@avelad
Copy link
Member Author

avelad commented Mar 4, 2020

With this change is not necessary a promise (for me, i don't know other use cases...)

@avelad
Copy link
Member Author

avelad commented Mar 4, 2020

image

@joeyparrish , Why do you appear as the creator of the commit?

57ce383

@joeyparrish
Copy link
Member

I'm not sure. I just clicked the merge button in GitHub's PR interface.

@joeyparrish
Copy link
Member

Looking at the details in the git log, it looks like GitHub did something odd:

commit 57ce383b389467bf7cf82ae10ad9f93c75dfecd1
Author: Joey Parrish <joeyparrish@users.noreply.github.com>
Commit: GitHub <noreply@github.com>

Normally, I would expect to see you as the author, as I do in this one:

commit c6323248803b590713bfdf3cf06194c0c3c13059
Author: Álvaro Velad Galván <alvaro.velad@mirada.tv>
Commit: GitHub <noreply@github.com>

Perhaps you should raise an issue with GitHub support. I'm not sure what I could have done differently on my end.

@joeyparrish
Copy link
Member

It looks like this has happened before, but not recently:

$ git log --pretty=full | grep -B 1 'Commit: GitHub' | grep Author
Author: Joey Parrish <joeyparrish@users.noreply.github.com>
Author: Tomohiro Matsuzawa (Tomo) <thmatuza75@hotmail.com>
Author: Álvaro Velad Galván <alvaro.velad@mirada.tv>
Author: Álvaro Velad Galván <alvaro.velad@mirada.tv>
Author: Álvaro Velad Galván <alvaro.velad@mirada.tv>
Author: Brian Harris <brianfromoregon@gmail.com>
Author: Álvaro Velad Galván <alvaro.velad@mirada.tv>
Author: Álvaro Velad Galván <alvaro.velad@mirada.tv>
Author: Álvaro Velad Galván <alvaro.velad@mirada.tv>
Author: Álvaro Velad Galván <alvaro.velad@mirada.tv>
Author: Álvaro Velad Galván <alvaro.velad@mirada.tv>
Author: Tijn Porcelijn <porcelijn@gmail.com>
Author: Álvaro Velad Galván <alvaro.velad@mirada.tv>
Author: Álvaro Velad Galván <alvaro.velad@mirada.tv>
Author: LanaIV <LanaIV@users.noreply.github.com>
Author: Álvaro Velad Galván <alvaro.velad@mirada.tv>
Author: Álvaro Velad Galván <alvaro.velad@mirada.tv>
Author: Joey Parrish <joeyparrish@users.noreply.github.com>
Author: Joey Parrish <joeyparrish@users.noreply.github.com>
Author: Joey Parrish <joeyparrish@users.noreply.github.com>
Author: Joey Parrish <joeyparrish@users.noreply.github.com>
Author: Joey Parrish <joeyparrish@users.noreply.github.com>
Author: Joey Parrish <joeyparrish@users.noreply.github.com>
Author: Joey Parrish <joeyparrish@users.noreply.github.com>
Author: Joey Parrish <joeyparrish@users.noreply.github.com>
Author: Joey Parrish <joeyparrish@users.noreply.github.com>
Author: Joey Parrish <joeyparrish@users.noreply.github.com>
Author: Joey Parrish <joeyparrish@users.noreply.github.com>
Author: Joey Parrish <joeyparrish@users.noreply.github.com>
Author: Joey Parrish <joeyparrish@users.noreply.github.com>
Author: Joey Parrish <joeyparrish@users.noreply.github.com>
Author: Joey Parrish <joeyparrish@users.noreply.github.com>
Author: Joey Parrish <joeyparrish@users.noreply.github.com>
Author: Joey Parrish <joeyparrish@users.noreply.github.com>
Author: Joey Parrish <joeyparrish@users.noreply.github.com>
Author: Joey Parrish <joeyparrish@users.noreply.github.com>
Author: Joey Parrish <joeyparrish@users.noreply.github.com>

And if you dig, there are even more PRs that don't show GitHub as the commiter:

commit 6e8ab0132fa2b04d5cfc15a18165ec7c0a9b6a83
Author: Álvaro Velad Galván <alvaro.velad@mirada.tv>
Commit: Joey Parrish <joeyparrish@users.noreply.github.com>
commit c6f0a86d7399c8fa0fa08a5f3e8cc3aa7e13834e
Author: Leon Chen <leonhart.chen@gmail.com>
Commit: Joey Parrish <joeyparrish@users.noreply.github.com>

I'm sorry I don't understand what's happening. We would prefer you have full credit in the commit log, of course. You've contributed a lot to this project, and we're very grateful.

@avelad
Copy link
Member Author

avelad commented Mar 4, 2020

I just wanted to know what could have happened. No problem in not appearing as an author, I prefer to have the functionality :)

Thanks!

@joeyparrish
Copy link
Member

We use the "squash & merge" feature in this repo, so it's possible that has something to do with it. But I don't see a difference in behavior that I could tie to squashing. For example, #2400 (2 commits) and #2408 (1 commit) both have the correct info.

In any case, thanks again for your contributions!

@joeyparrish joeyparrish self-assigned this Mar 4, 2020
@avelad
Copy link
Member Author

avelad commented Mar 4, 2020

With the new commit 7b80252 there is the same issue

@joeyparrish
Copy link
Member

Yes, I noticed that. I went through the dialog carefully that time, and there wasn't anything different I could have done as far as I can tell.

@avelad avelad deleted the loaded-event branch March 5, 2020 07:54
joeyparrish added a commit that referenced this pull request Mar 18, 2020
Backported to v2.5.x

Change-Id: I47c2f19fff39b8be737e7c45b71b1026f8a5bdf0
@github-actions github-actions bot added the status: archived Archived and locked; will not be updated label Jul 25, 2023
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jul 25, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

status: archived Archived and locked; will not be updated

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants