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

Restore novtt version of VideoJS #2447

Closed
wants to merge 1 commit into from

Conversation

misteroneill
Copy link
Member

The novtt version of VideoJS was not being created (though there was a copy task for it). This restores it and moves the generation of banners out of the browserify process so that the license banner can be programmatically generated depending on the presence of vtt.js. Relevant changes are:

  • Lodash is pulled into the Gruntfile, but it was already a dependency.
  • License banner templates are processed by customized processor functions, which manually render.

The novtt version of VideoJS was not being created (though there was a
`copy` task for it). This restores it and moves the generation of
banners out of the browserify process so that the license banner can
be programmatically generated depending on the presence of vtt.js.

- Lodash is pulled into the Gruntfile, but it was already a dependency.
- License banner templates are processed by customized processor
  functions, which manually render.
@dmlap
Copy link
Member

dmlap commented Aug 10, 2015

lgtm

@gkatsev
Copy link
Member

gkatsev commented Aug 10, 2015

LGTM

@dmlap dmlap closed this in c0673d2 Aug 10, 2015
@misteroneill misteroneill deleted the restore-novtt branch August 11, 2015 14:06
options: {
separator: '\n'
},
src: ['build/temp/video.js'],
Copy link
Member

Choose a reason for hiding this comment

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

We're not actually concatenating anything here, but works for me. :)

@heff
Copy link
Member

heff commented Aug 12, 2015

fwiw, I don't think the separate license header for vtt is really necessary, but nicely done either way.

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

Successfully merging this pull request may close these issues.

4 participants