ome / omero-insight

Gradle project containing insight java client for OMERO
https://www.openmicroscopy.org/omero/
GNU General Public License v2.0
7 stars 14 forks source link

use gradle action #349

Closed jburel closed 1 year ago

jburel commented 1 year ago

Using the Gradle action installed the desired version of gradle preventing the failure. see https://github.com/ome/omero-insight/actions/runs/4202376704/jobs/7353445683 TODO: review other repositories

Check that the build is green

dominikl commented 1 year ago

👍 Looks good. These github actions seem to be quite a pain to maintain...

jburel commented 1 year ago

It is not really a pain, things evolved as the quality of the tooling increases. the gradle-action used her was not around (or I was not aware of it) when we first started using them

sbesson commented 1 year ago

As we might be looking into upgrading Gradle to 7.x across the board, is there a track record of the rationale and the initial decision not to use the Gradle Wrapper across the new Java repositories?

Obviously there are advantages and drawbacks to either approach. In our case, the Jenkins and the super-module infrastructure are likely implemented without this assumption and would require modification. One advantage from a developer perspective is that it declares the Gradle version in an explicit manner. At the CI level, I assume this would remove the requirement to manage the Gradle version both in the GitHub workflow and within the Jenkins node, reducing the need for PRs such as this one.

jburel commented 1 year ago

Other repositories use https://github.com/ome/action-gradle/pull/2 Using the wrapper is also an option with that action. But as we have noticed before using the wrapper can lead to error during the "installation" of the wrapper see for example https://github.com/ome/omero-insight/pull/220

This is exactly what happens here with Gradle 8, some "check-up" tasks are run with the current release (gradle 8) before actually using the wrapper.

Note that Jenkins does not use the "wrapper" approach. It currently uses Gradle 5 preventing the introduction of Gradle 7 support

We used to have the wrapper has part of the repositories. This was discussed and removed due to maintenance issue.

Edit: the wrapper approach via that action has not been tested and might work

jburel commented 1 year ago

We will revisit the strategy post release i.e. include or not gradlew file. Merging so we can build the release