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
Add support for webpack2 #50 #57
Conversation
Can one of the admins verify this patch? |
test this please |
Refer to this link for build results (access rights to CI server needed): |
I don’t understand why has Jenkins checked out the wrong branch… |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let me know if you can apply the requested changes, otherwise I will do it :)
case e: Exception => None | ||
} | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think you could rewrite this as follows:
version match {
case r(v, _) => Try(v.toInt).toOption
case _ => None
}
Some(json.fromJSON[NpmPackage](json.readJSON(new FileReader(webpackPackageJsonFilePath)))) | ||
} catch { | ||
case e : Exception => None | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I prefer Try(…).toOption
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Will do it, min
d023342
to
3199422
Compare
We've again got the error: "Error: Cannot find module 'C:\Users\appveyor\AppData\Local\Temp\1\sbt_492497ae\static2\target\scala-2.11\node_modules\webpack\bin\webpack'". @julienrf how did you fix it the first time? |
I didn’t fix this issue yet :( |
fe7742e
to
b966319
Compare
The PR is great but, I don’t know why, there is an issue in AppVeyor: some files are not correctly removed by the |
So, I’m going to merge the PR to an internal |
Please review :)