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

use tarball dependency of vtt.js #1911

Closed
wants to merge 1 commit into from

Conversation

silverwind
Copy link

This uses the tarball download instead of git to obtain the vtt.js fork, fixing my issue encountered in #1895.

Technically, there is no difference, except that npm apparently doesn't strip down the dependency as defined by files when downloaded as tarball, but that seems like a npm bug of its own.

@heff
Copy link
Member

heff commented Mar 2, 2015

Seems ok to me. @gkatsev?

@gkatsev
Copy link
Member

gkatsev commented Mar 2, 2015

I don't have an issue per-se other than it's lame that cygwin ships a broken git implementation. Assuming that this is pointing at the correct branch of my fork, it's fine.

@silverwind
Copy link
Author

I'll dig deeper once time permits to get to the root of that Cygwin issue.

@gkatsev
Copy link
Member

gkatsev commented Mar 2, 2015

Also, possibly an npm issue as well.

@silverwind
Copy link
Author

npm does execute the correct command at least, it's probably the shell or git itself that wrongly joins the c:\\-style paths when cloning from local to local repo.

@gkatsev
Copy link
Member

gkatsev commented Jul 8, 2015

I'm going to close this one, especially since I don't want to point directly at a branch anymore and we have a new change for vttjs (#2327). Hopefully soon we'll be able to just use the official vttjs npm repo or I could always just publish a vjs-vtt-only fork onto npm.

@gkatsev gkatsev closed this Jul 8, 2015
@silverwind
Copy link
Author

Fine with me, given this is more of a personal issue. Looking forward to you using the official package soon.

@silverwind silverwind deleted the tarball-dependency branch July 25, 2015 05:04
@gkatsev
Copy link
Member

gkatsev commented Dec 9, 2015

Fixed by #2905

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

Successfully merging this pull request may close these issues.

3 participants