You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@maven.apache.org by GitBox <gi...@apache.org> on 2022/06/05 20:38:12 UTC
[GitHub] [maven-fluido-skin] stevecrox opened a new pull request, #32: Mskins 191 dynamic library retrieval
stevecrox opened a new pull request, #32:
URL: https://github.com/apache/maven-fluido-skin/pull/32
I have modified this branch so the repository no longer contains the JS files used, instead we make use of the com.github.eirslett:frontrend-plugin.
This plug-in will pull down a copy of Node.JS into the build directory, this in turn allows us to call 'npm' and install bootstrap and anchor-js via NPM (since NPM Regsitry is how front end developers "release" libraries). This is performed in the 'generate-resources' phase.
I had to bump the bootstrap version from 2.8.3 to 3.1.1, the classes uses dont appear to have changed, the big change was the removal of jQuery so this was removed from the project.
I then added several executions to the maven-resources-plugin, these run in the 'process-resources' (phase after 'generate-resources'). The com.samaxes.maven:minify-maven-plugin plugin expects all the CSS/JS files to be in the same root directory, they are obviously now in different directories (e.g. src/main/resources and target/node_modules). So we use maven-resources-plugin to move them into a single directory (e.g. target/interim-minify). This lets the minify plug-in create the same build artefacts as before.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven-fluido-skin] hboutemy commented on pull request #32: Mskins 191 dynamic library retrieval
Posted by GitBox <gi...@apache.org>.
hboutemy commented on PR #32:
URL: https://github.com/apache/maven-fluido-skin/pull/32#issuecomment-1147281228
I love the idea of making updates of the 3 used js libraries easier, even if the hard work when upgrading is to check that the template still works as expected: at least, it eases having a quick look
like Michael, I feel that introducing npm for that looks too heavy
Could an approach like webjars do the same job? see https://github.com/webjars/bootstrap/blob/master/pom.xml
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven-fluido-skin] rmannibucau commented on pull request #32: Mskins 191 dynamic library retrieval
Posted by GitBox <gi...@apache.org>.
rmannibucau commented on PR #32:
URL: https://github.com/apache/maven-fluido-skin/pull/32#issuecomment-1147312549
@hboutemy exploding webjars and copying resources would work but would still rely on outdated plugins for the resources build, this is where using nodejs ecosystem would be a big plus for the theme until we go with CDN as mentionned which would make the game pretty different then. So, IMHO, to summarize it is not only about fetching but more about having a proper front build where node helps.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven-fluido-skin] stevecrox closed pull request #32: Mskins 191 dynamic library retrieval
Posted by GitBox <gi...@apache.org>.
stevecrox closed pull request #32: Mskins 191 dynamic library retrieval
URL: https://github.com/apache/maven-fluido-skin/pull/32
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven-fluido-skin] hboutemy commented on pull request #32: Mskins 191 dynamic library retrieval
Posted by GitBox <gi...@apache.org>.
hboutemy commented on PR #32:
URL: https://github.com/apache/maven-fluido-skin/pull/32#issuecomment-1147290751
notice: given updates to the skin requires to check the rendering, I had a look at how easy currently
and I found:
1. it's easy to see the result of many edge cases: https://maven.apache.org/skins/maven-fluido-skin/ITs.html
2. but it's not so easy to see result for the sample page, given it uses previous skin version: https://maven.apache.org/skins/maven-fluido-skin/sample.html
perhaps we should inject the sample page content into the 3 main layouts tests https://maven.apache.org/skins/maven-fluido-skin/sidebar/index.html
we never had much contribution to skins, this is time to learn together how to improve that aspect
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven-fluido-skin] stevecrox commented on pull request #32: Mskins 191 dynamic library retrieval
Posted by GitBox <gi...@apache.org>.
stevecrox commented on PR #32:
URL: https://github.com/apache/maven-fluido-skin/pull/32#issuecomment-1147355223
I am interested in this skin, because maven allows me to incorporate documentation and analysis into a site. The problem is the current site style has dated.
Historically full stack development has JS/HTML/CSS stored in an appropriate resources directory of a java project. This made developing frontends painful.
Node.js/NPM appeared 10 years ago and you could build a mini webserver fairly easy with express and iterate there was much joy. We hit the problem I am addressing in this PR.
The format NPM releases a project is different to Maven.
There are several common approaches to integrate the UI with the backend
1) write a release time script which calls Maven and generates a JAR when the NPM project releases.
2) Use the latest java wrapper for JS
3) Write the front end so its hosted statically in its own webserver and the UI calls to the backend
4) Use the front end plugin to pull down the dependecies
The problem with 1 is what we see here and requires bootstrap to make a change to support you.
The problem with 2 (e.g. webjars) is Web Development is happy in its ecosystem. So adoption always fails (Personally I loved the maven extension that let me package js).
Approach 3 isn't valid here, fundamentally what this skin offers are some resources and a template file to apache velocity.
The last approach is what I have chosen, we use the frontend plugin to retrieve node/npm which are stored in a temporary directory. We use NPM to retrieve the releases and then the minify plugin to generate the JS artifacts.
The result is no additional dependencies for users of the skin, there should be zero change from their perspective.
I saw the comment on CDN, I have always worked in defense companies. Defense company infosec departments are not reasonable (and iften non technical) meaning this sort of stuff is developed in effectively air gapped environments and I have spent way to much of my life designing and pushing for mirrors of real world infrastructure. One of the reasons I love Maven is as a toolit makes redirecting to local mirrors really easy.
As for why this change?
I have a bootstrap 5 skin supporting a fixed navbar top but its a complete rewrite. Bootstrap has its examples and then there are HTML layouts
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven-fluido-skin] slachiewicz commented on pull request #32: Mskins 191 dynamic library retrieval
Posted by GitBox <gi...@apache.org>.
slachiewicz commented on PR #32:
URL: https://github.com/apache/maven-fluido-skin/pull/32#issuecomment-1147321586
Main challenge is how to cut template, adjust macros.
including 2 css and .js files doesn't have to be problem here.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven-fluido-skin] michael-o commented on pull request #32: Mskins 191 dynamic library retrieval
Posted by GitBox <gi...@apache.org>.
michael-o commented on PR #32:
URL: https://github.com/apache/maven-fluido-skin/pull/32#issuecomment-1147172114
The need for an upgrade does not relate to using npm or packaging. Maven sites are supposed to be self-contained because they can be archived, see site:jar. Using CDNs would completely break this. What is the real world benefit of this smart-ass minification? Does it really matter for this simple skin?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven-fluido-skin] rmannibucau commented on pull request #32: Mskins 191 dynamic library retrieval
Posted by GitBox <gi...@apache.org>.
rmannibucau commented on PR #32:
URL: https://github.com/apache/maven-fluido-skin/pull/32#issuecomment-1147350603
Not sure, using node today is not rocket science (at least no more than using velocity to create a template) but overall it would benefit end users (either using CDN or node to build the bundle). Can avoid the build postprocessing to rewrite the resources in site deployment which care - but I can also hear it is less and less used outside apache so maybe not worth it if we don't care for ourselves.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven-fluido-skin] michael-o commented on pull request #32: Mskins 191 dynamic library retrieval
Posted by GitBox <gi...@apache.org>.
michael-o commented on PR #32:
URL: https://github.com/apache/maven-fluido-skin/pull/32#issuecomment-1147325263
I don't want this skin to be rocket science, but this makes it.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven-fluido-skin] rmannibucau commented on pull request #32: Mskins 191 dynamic library retrieval
Posted by GitBox <gi...@apache.org>.
rmannibucau commented on PR #32:
URL: https://github.com/apache/maven-fluido-skin/pull/32#issuecomment-1147111762
Some comments because I think the issue is legitimate and we are very 'back' on the frontend deliveries for now.
Maybe using download maven plugin or equivalent to grab the resources from a CDN can be sufficient for now but I think we should consider:
1. NOT packagng the resources in the jar, in particular because we don't own them and we should and will need to upgrade for various reasons (starting by the fact these versions are no more supported and have vulnerabilities)
2. use node to handle the minification instead of a plugin which does not handle some optimizations of modern web (using browserify at least) sounds like a very good choice for the final delivery so can be worth keeping frontend plugin
3. (optional) enable to use CDN in the theme which would be probably the best option for most people anyway
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven-fluido-skin] michael-o commented on pull request #32: Mskins 191 dynamic library retrieval
Posted by GitBox <gi...@apache.org>.
michael-o commented on PR #32:
URL: https://github.com/apache/maven-fluido-skin/pull/32#issuecomment-1147105353
I am rejecting this PR for the following reasons:
* npm/nodejs bring a huge dependency chain for a simple problem
* we don't want to rely on external tools
* we rarely change those dependencies
I see very little benefit in doing this for us.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org