Conversation
There was a problem hiding this comment.
A little nit-pick: resin-java8 would conform a bit better to install file conventions.
Convention is also to have the install name match the .installed name. There may be issues since resinJava8 != resin-4.0.41. Not catastrophic, but this will cause the toolset to always install resinJava8 even if it is already installed.
Recommendation: use resin-java8 and touch resin-java8.installed on this commented line.
frameworks/Java/tapestry/install.sh
Outdated
There was a problem hiding this comment.
Agreed. If we have to lock down the java version prior to installing resin, it would be better to just remove the line fw_depends java7 from resin.sh and rely on the caller explicitly setting the java version desired via fw_depends java7 resin or fw_depends java8 resin, instead of creating resin-java8.sh. Perhaps we could replace fw_depends java7 with a check that either java7.installed or java8.installed exists and fail if that is not true just as one sanity check
There was a problem hiding this comment.
We discussed this offline and apparently resin needs to be compiled with the same java runtime that is used to invoke it. So, this resin-java8 must exist unless we move EVERYTHING to java8 explicitly. Very annoying.
There was a problem hiding this comment.
This file needs to be removed completely.
|
looks like a lot of unrelated tests failed in travis. |
blee-techempower
force-pushed
the
master
branch
from
05fbfd0 to
d5b976a
Compare
|
Localtest failed. ( http://sprunge.us/fUNM ) |
There was a problem hiding this comment.
These two lines need to be replaced by fw_depends java8 resin-java8
|
@ssmith-techempower See if you can make a branch off of this PR and get it updated to work. EDIT: You will want to rebase off our master, as well. |
|
Unrelated: does anyone know why this PR is called "Based2 patch 2"? EDIT: Figured it out; it means to fix up #1547 started by user |
blee-techempower
force-pushed
the
master
branch
from
ddbc264 to
c948188
Compare
|
Closing in favor of #1816 |
9 participants
No description provided.