-
Notifications
You must be signed in to change notification settings - Fork 213
Update gradle documentation, split it into two pages. #4669
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
base: v24
Are you sure you want to change the base?
Conversation
AI Language ReviewThe updated document streamlines content by removing some detailed sections and focusing on essential tasks for creating, compiling, and running a Vaadin project with Gradle. It simplifies the requirements section, improves consistency in describing Gradle tasks, and updates Gradle and plugin versions.
Overall, the revisions provide a more user-friendly guide but ensure that readers can easily find detailed references as needed, especially when specific configurations are omitted. |
02d9411 to
72a3d41
Compare
afd1b32 to
7718ac0
Compare
|
I think the remaining AI Language Review suggestions can be ignored, I think it's unaware that the original document has been split into two, and more focused / less-verbose approach of the document is desired. |
|
|
||
| .Requirements | ||
| [NOTE] | ||
| The Vaadin Gradle plugin requires Java SDK 17 or later, Gradle 8.5 or later. |
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.
Looking at the Vaadin 24.8 and 24.9 release notes, the minimum Gradle version is 8.7
| The Vaadin Gradle plugin requires Java SDK 17 or later, Gradle 8.5 or later. | |
| The Vaadin Gradle plugin requires Java SDK 17 or later, Gradle 8.7 or later. |
| .Requirements | ||
| [NOTE] | ||
| The Vaadin Gradle plugin requires Java SDK 17 or later, Gradle 8.5 or later. | ||
| It will install Node.js and npm automatically when running the build for the first time. |
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.
It could be worth it adding that install happens only if Node is not already installed or if it does not match the minimum version required by Vaadin.
|
|
||
| ---- | ||
| vaadin { | ||
| optimizeBundle = false |
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'm not sure this option is needed for development. Ideally, for development there should be no need for any configuration.
What about changing the For development, the block is usually like this with something like An example configuration could be like this and use frontendHotdeploy = true in the snippet?
| } | ||
| ---- | ||
|
|
||
| If the parameter is `true`, the frontend bundle is optimized for all supported browsers, but the compilation is much slower. For configuration options, see <<all-options,plugin configuration options>> |
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.
optimizeBundle is to enable the byte code scanner and make the bundle only contain frontend stuff that is actually used in the application. It has nothing to do with browser optimizations.
| * repository for *Vaadin add-ons* | ||
| * repository for *Vaadin pre-releases* - not recommended for production environments | ||
| * *Gradle plugin repository* - only necessary when using Gradle community plugins, which are not available through Maven central | ||
|
|
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.
What about also mention mavenLocal() for artifact published only locally? However, this would probably make sense only when working on projects with mixed Maven and Gradle build tools.
| } | ||
| ---- | ||
|
|
||
| When you specify a version of `24.+`, you're choosing to use the latest version of Vaadin. However, you can also specify the exact version. See https://docs.gradle.org/current/userguide/declaring_dependencies.html[Declaring Dependencies] in the Gradle documentation for more details. |
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.
nit: I'm not sure if we should suggest using an unpinned version. IMO, from a security point of view, I would always pin the exact version I want to use.
But this is not a blocker.
| The Vaadin-related tasks handled by the plugin are as follows: | ||
|
|
||
| `vaadinPrepareFrontend`:: | ||
| This checks that Node.js and `npm` are installed, copies frontend resources, and creates or updates the [filename]`package.json` and Vite configuration files (i.e., [filename]`vite.config.ts` and [filename]`vite.generated.ts`). The frontend resources are inside `.jar` dependencies: they're copied to `node_modules`. |
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.
It looks like the Javadoc for Maven and Gradle prepare frontend task is outdated.
Looking at the prepare fronted code I can see the following
.createMissingPackageJson(true).enableImportsUpdate(false)
.enablePackagesUpdate(false).withRunNpmInstall(false)So it seems that prepare frontend:
- creates package.json is necessary but does not update it
- resources inside JAR files are copied into
src/main/frontend/generated/jar-resources, not innode_modules
| dependencies { | ||
| classpath group: 'com.vaadin', | ||
| name: 'vaadin-gradle-plugin', | ||
| version: '24.0-SNAPSHOT' | ||
| } |
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'm not so skilled in Gradle, but at least our starter seems to provide a different plugin configuration.
It has the version pinned in settings file (https://github.com/vaadin/base-starter-gradle/blob/9a8284e07d02e0f6f346b49accbebd902e50865a/settings.gradle#L7) and only definition in the build script (https://github.com/vaadin/base-starter-gradle/blob/9a8284e07d02e0f6f346b49accbebd902e50865a/build.gradle#L15), no apply at all.
So the question is if it would be enough to set the snapshot version in settings in combination with the configuration of the pre-release repository.
I'm not sure what we should suggest in the docs. Maybe @mvysny can provide some insight
Solves #4222
Changes: