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

Convert video.{width,height} from strings to numbers #453

Closed
wants to merge 1 commit into from
Closed

Convert video.{width,height} from strings to numbers #453

wants to merge 1 commit into from

Conversation

hochhaus
Copy link
Contributor

No description provided.

@joeyparrish joeyparrish added the type: enhancement New feature or request label Jul 24, 2016
@joeyparrish joeyparrish self-assigned this Jul 24, 2016
@shaka-bot
Copy link
Collaborator

Testing in progress...

@joeyparrish joeyparrish added type: bug Something isn't working correctly and removed type: enhancement New feature or request labels Jul 24, 2016
@shaka-bot
Copy link
Collaborator

Failure:

+ echo START-BUILD
START-BUILD
+ ./build/all.py
157 files checked, no errors found.

   Config loaded: �[36m/var/lib/jenkins/jobs/Manual PR Test/workspace/.htmlhintrc�[39m

   Config loaded: �[36m/var/lib/jenkins/jobs/Manual PR Test/workspace/.htmlhintrc�[39m

   Config loaded: �[36m/var/lib/jenkins/jobs/Manual PR Test/workspace/.htmlhintrc�[39m

�[32mScan 3 files, without errors (34 ms).�[39m
/var/lib/jenkins/jobs/Manual PR Test/workspace/test/media/drm_engine_integration.js:55: ERROR - assignment to property width of HTMLVideoElement
found   : number
required: string
    video.width = 600;
    ^

/var/lib/jenkins/jobs/Manual PR Test/workspace/test/media/drm_engine_integration.js:56: ERROR - assignment to property height of HTMLVideoElement
found   : number
required: string
    video.height = 400;
    ^

/var/lib/jenkins/jobs/Manual PR Test/workspace/test/media/media_source_engine_integration.js:60: ERROR - assignment to property width of HTMLVideoElement
found   : number
required: string
    video.width = 600;
    ^

/var/lib/jenkins/jobs/Manual PR Test/workspace/test/media/media_source_engine_integration.js:61: ERROR - assignment to property height of HTMLVideoElement
found   : number
required: string
    video.height = 400;
    ^

/var/lib/jenkins/jobs/Manual PR Test/workspace/test/media/streaming_engine_integration.js:75: ERROR - assignment to property width of HTMLVideoElement
found   : number
required: string
    video.width = 600;
    ^

/var/lib/jenkins/jobs/Manual PR Test/workspace/test/media/streaming_engine_integration.js:76: ERROR - assignment to property height of HTMLVideoElement
found   : number
required: string
    video.height = 400;
    ^

/var/lib/jenkins/jobs/Manual PR Test/workspace/test/offline/offline_integration.js:28: ERROR - assignment to property width of HTMLVideoElement
found   : number
required: string
    video.width = 600;
    ^

/var/lib/jenkins/jobs/Manual PR Test/workspace/test/offline/offline_integration.js:29: ERROR - assignment to property height of HTMLVideoElement
found   : number
required: string
    video.height = 400;
    ^

/var/lib/jenkins/jobs/Manual PR Test/workspace/test/player_integration.js:36: ERROR - assignment to property width of HTMLVideoElement
found   : number
required: string
    video.width = 600;
    ^

/var/lib/jenkins/jobs/Manual PR Test/workspace/test/player_integration.js:37: ERROR - assignment to property height of HTMLVideoElement
found   : number
required: string
    video.height = 400;
    ^

10 error(s), 0 warning(s), 90.4% typed
Build failed
Generating Closure dependencies...
Running Closure linter...
Running htmlhint...
Checking that the build files are complete...
Checking the tests for type errors...
Build step 'Execute shell' marked build as failure

@hochhaus
Copy link
Contributor Author

hochhaus commented Jul 24, 2016

It looks like this change depends on the updated compiler. Sorry for the confusion. I didn't realize how recently google/closure-compiler@91014522 was merged.

Would you like me to squash this into #421 or do you just want to merge them at the same time?

@joeyparrish
Copy link
Member

No worries. Just make this part of the same PR with the new compiler itself.

@hochhaus hochhaus deleted the string_number 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.
Labels
status: archived Archived and locked; will not be updated type: bug Something isn't working correctly
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants