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

Fix JSDoc annotations #458

Merged
merged 1 commit into from
Jul 24, 2016
Merged

Fix JSDoc annotations #458

merged 1 commit into from
Jul 24, 2016

Conversation

hochhaus
Copy link
Contributor

No description provided.

@hochhaus hochhaus mentioned this pull request Jul 24, 2016
Merged
@joeyparrish joeyparrish self-assigned this Jul 24, 2016
joeyparrish
joeyparrish reviewed Jul 24, 2016
View reviewed changes
test/media/streaming_engine_integration.js
@@ -522,6 +522,7 @@ describe('StreamingEngine', function() {
* Initializes or switches to the given period.
*
* @param {shakaExtern.Period} period
* @return {{audio: shakaExtern.Stream, video: shakaExtern.Stream}}
Copy link
Member

@joeyparrish joeyparrish Jul 24, 2016 •
edited
Loading

Choose a reason for hiding this comment

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

I think this should be {!Object.<string, shakaExtern.Stream>}

@hochhaus
Copy link
Contributor Author

Thanks. Updated per your comment.

@hochhaus
Copy link
Contributor Author

I actually updated again to @return {!Object.<string, !shakaExtern.Stream>} as that type is slightly tighter. Please let me know if you disagree.

joeyparrish
joeyparrish reviewed Jul 24, 2016
View reviewed changes
test/media/streaming_engine_integration.js
@@ -522,6 +522,7 @@ describe('StreamingEngine', function() {
* Initializes or switches to the given period.
Copy link
Member

Choose a reason for hiding this comment

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

This comment is out of date. Can you please update to "Choose streams for the given period"? Same in player_unit and streaming_engine_unit.

@hochhaus
Copy link
Contributor Author

Updated the comments per your feedback.

PTAL

@joeyparrish
Copy link
Member

Since shakaExtern.Stream is a typedef of a record type, it is already non-nullable by default, but there's nothing wrong with adding the exclamation point.

@hochhaus
Copy link
Contributor Author

it is already non-nullable by default

Thanks for the explanation. I didn't realize that.

@joeyparrish
Copy link
Member

Yeah, sometimes Closure is more subtle than I would like.

@shaka-bot
Copy link
Collaborator

Testing in progress...

@shaka-bot
Copy link
Collaborator

All tests passed!

@joeyparrish joeyparrish merged commit 4ccaf23 into shaka-project:master Jul 24, 2016
@hochhaus hochhaus deleted the annotations branch July 26, 2016 20:57
@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.
Reviewers
No reviews
Assignees

@joeyparrish joeyparrish

Labels
status: archived Archived and locked; will not be updated
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@hochhaus @joeyparrish @shaka-bot