-
Notifications
You must be signed in to change notification settings - Fork 80
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
Provide docker-web project for gradle, restructure web static content #1694
Conversation
def ideClientJsJar = tasks.register('ideClientJsJar', Jar) { | ||
from configurations.js | ||
} |
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.
The jar name is different than before. deephaven-web-client-ide-0.8.0-js.jar
vs web-0.8.0.jar
. Since it was a under the guise of a java project before, it inherited buildSrc/src/main/groovy/io.deephaven.java-conventions.gradle
which includes setting archivesBaseName
. The easy fix is to set
archivesBaseName = "deephaven-web"
. Long term, we should probably have base-conventions.gradle
separated out.
Do we need/want the -js
classifier on the jar name?
Upon my first build of web:ideClientJsJar
, I was seeing extra content. It may have been old gradle directories that were being passively picked up. Was a bit strange. After a clean it went away.
Here are the jar tf
for the builds:
$ ls -la /tmp/*.txt
-rw-r--r--. 1 devin devin 4943 Dec 14 13:50 /tmp/deephaven-web-client-ide-0.8.0-js.txt
-rw-r--r--. 1 devin devin 4667 Dec 14 13:56 /tmp/web2.txt
-rw-r--r--. 1 devin devin 8336 Dec 14 13:51 /tmp/web.txt
You can see the first web build is about twice as big as web2.
And as far as content, I see that there are some different prefixes now.
For example:
$ grep wordmark /tmp/*.txt
/tmp/deephaven-web-client-ide-0.8.0-js.txt:dhide/static/media/community-wordmark-app.1acb5a93.svg
/tmp/web2.txt:ide/static/media/community-wordmark-app.1acb5a93.svg
$ grep treegrid /tmp/*.txt
/tmp/deephaven-web-client-ide-0.8.0-js.txt:dhapi/treegrid.js
/tmp/web2.txt:jsapi/treegrid.js
dhapi
-> jsapi
and dhide
-> ide
, that's correct?
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.
The jar name is different
Do we care? I'm not sure either way just yet. This jar is being created only so that it can be on the classpath of the eventual jetty-based executable, so that content can be served off of its classpath.
If we publish the web ui's static content as a jar to central I'm sure we care. Otherwise I'm uncertain, will be guided by you.
Do we want/need the -js classifier
No, this project does not produce java that is useful downstream to anyone (at this time?)
--
Probably your uncleaned version had leftover UI stuff copied into it - I've seen that a lot recently where gradle is over-confident in what is correctly already built and what isn't. 99% the "fault" is actually mine, but it is very hard to reliably use when it is so hard to hold the tool correctly. I'll take another pass at checking the before/after of this.
--
dhapi -> jsapi and dhide -> ide, that's correct?
"jsapi" and "ide" are the desired paths that the user sees in the browser. We can probably rewrite the "dhapi" and "dhide" internally, but these path changes are consistent with the build before my patch. If you'd like, I'll address that now, your call.
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 do care about about the jar name prefix - deephaven-<x>
, I don't care about the classifier being appended.
Please set the archiveBaseName to deephaven-web
(or something similar if you think it makes more sense), but put a reference in the code to #1701
6cf6a94
to
8683ee9
Compare
8683ee9
to
c5b74e9
Compare
No description provided.