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

Vinyl #120

Merged
merged 31 commits into from
Oct 17, 2016
Merged

Vinyl #120

merged 31 commits into from
Oct 17, 2016

Conversation

bezoerb
Copy link
Collaborator

@bezoerb bezoerb commented Jan 28, 2016

This is a bigger maintenance PR. It includes the following improvements:

We now can campute the path prefix for asset references based on the location of the html file.
This allows us to keep relative asset urls in stylesheets relative. Issues #57 and #95
try to guess source path of the file by checking relative links inside the html source
show warning to the user when no relative links are available
some weird issue requires a second npm install to add all deps
without this module minimist required by gulp-util is missing
@bezoerb
Copy link
Collaborator Author

bezoerb commented Jan 29, 2016

still needs some work

@Tempurturtul
Copy link

In index.js:

exports.stream = function (opts) {
    // ...
        var options = _.assign(opts || {}, {
            // src: file
            src: file.path
        });
    // ...

Seems to provide correct behavior in test cases.

Should I open a new pull request? I don't know the etiquette here and I greatly appreciate how quickly you worked on this. 👍

@bezoerb
Copy link
Collaborator Author

bezoerb commented Feb 8, 2016

@addyosmani wanna take a look?

@addyosmani addyosmani self-assigned this Feb 26, 2016
@addyosmani
Copy link
Owner

Yep. Sorry I've been out of town for a few weeks but I'll take a look. Thanks for working on this ⭐

@bezoerb
Copy link
Collaborator Author

bezoerb commented Feb 29, 2016

Still needs some work (see #57)
Asset paths should be computed relative to dest instead of the src path.
When no dest is available we need a target path option which should default to the src path

@bezoerb
Copy link
Collaborator Author

bezoerb commented Sep 28, 2016

\o/ green again :)

Added destFolder option used to compute the relative asset path.
Asset paths are now computed in this order:

  • relative to destFolder if option is set
  • relative to dest if option is set
  • relative to src file

@addyosmani: Want to take a look again? (Sorry for the 31 changed files ;))

@addyosmani
Copy link
Owner

Thanks for working on this! I'll take a look in the morning :)

@addyosmani
Copy link
Owner

The list of changes here on the whole LGTM. To avoid blocking any further work here lets land it.

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.

3 participants