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

Correct use of promises. #131

Closed
wants to merge 1 commit into from
Closed

Correct use of promises. #131

wants to merge 1 commit into from

Conversation

OJezu
Copy link

@OJezu OJezu commented Feb 3, 2015

fixes #118

Pull request is against v2.0.1 since the newest master broke something in include path resolution.

By using done I made sure that the promise chain won't be interrupted and either the error will be passed to cb or the process will crash. Which is still better than error vanishing.

@yocontra
Copy link
Member

yocontra commented Feb 3, 2015

Oh so render returns a promise all of the sudden? Interesting

@OJezu
Copy link
Author

OJezu commented Feb 3, 2015

Well, there was a .then(...) in v2.0.1 already, I just moved things a bit and added .done(). As far as I can tell, less (due to less/less.js#2430) threw an error in the onResolve function, which was the last in chain, so that error never was handled.

@yocontra
Copy link
Member

yocontra commented Feb 3, 2015

Can you rebase so this can merge? You made the changes on old code

@OJezu
Copy link
Author

OJezu commented Feb 3, 2015

As I said

Pull request is against v2.0.1 since the newest master broke something in include path resolution.

I.e.

Error in plugin 'gulp-less'
Message:
    'font-awesome/less/variables.less' wasn't found. Tried - /home/ch/src/Browser/style/font-awesome/less/variables.less,font-awesome/less/variables.less in file /home/ch/src/Browser/style/config.less line no. 1
Details:
    type: File
    filename: /home/ch/src/Browser/style/config.less
    index: 0
    line: 1
    callLine: NaN
    callExtract: undefined
    column: 0
    extract: ,@import "font-awesome/less/variables";,
    lineNumber: 1
    fileName: /home/ch/src/Browser/style/config.less

First guess would be something with lodash.defaults being replaced by object-assign.
Anyway, the same fix is at the top of OhJeez/gulp-less#master. I will make another pull request, but please be aware of the error above (which is not detected by unit tests).

@OJezu OJezu mentioned this pull request Feb 3, 2015
@OJezu OJezu mentioned this pull request Feb 11, 2015
@yocontra
Copy link
Member

Duplicate of #132

@yocontra yocontra closed this Feb 11, 2015
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.

Less compilation doesn't break on missing variable
2 participants