-
Notifications
You must be signed in to change notification settings - Fork 14
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
Gradle fix and publish #176
Conversation
👍 Seems to work fine: https://github.com/dominikl/omero-insight/releases/tag/v5.5.16 (I just forgot to commit the version change to 5.5.16 in the build.gradle). |
The msi did not get created
b/c you kept snapshot. The error is not reported as a failure. |
|
Failed with snapshot: https://github.com/dominikl/omero-insight/actions/runs/410215701 |
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.
All the changes generally make sense in order to produce packages, upload them as artifacts and combine them into release assets. A few questions inline but none is really a blocker to get this in.
Comparing https://github.com/dominikl/omero-insight/releases/tag/v5.5.18 generated with this PR with the last release https://github.com/ome/omero-insight/releases/tag/v5.5.14, is the removal of drop of omero_ij-5.5.14-all.jar
on purpose?
shell: bash | ||
run: echo "$WIX\\bin" >> $GITHUB_PATH | ||
- name: Build Executable | ||
if: startsWith(matrix.os, 'windows') && startsWith(github.ref, 'refs/tags') && matrix.java == '1.8' |
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.
Is build time the primary reason for making this condition to tags? Or the SNAPSHOT vs non-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.
this will increase the build time, that's why I have opted to make it on tag only
- name: Build Dmg | ||
if: startsWith(matrix.os, 'macos') && startsWith(github.ref, 'refs/tags') && matrix.java == '1.8' | ||
run: | | ||
brew cask install zulu8 |
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.
Arguably all these steps could be combined into a standalone script e.g. package_macos.sh
that gets executed by GitHub workflow but could be tested and modified in isolation.
Mostly a question, not a requirement for this PR
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.
This could be refactored now that we have the build in place.
This could be done in a follow-up PR
I must have dreamt that I saw the jar in the list of artifacts, it should be there. so another commit is required |
@sbesson This should now be fixed see https://github.com/jburel/omero-insight/releases/tag/v5.5.16 |
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.
Everything looks good with the last commit. I have not tested any of the artifacts so this might be something we will need to look at ahead of the next OMERO.insight release.
Sorry for the delay @jburel . Tested all artifacts of https://github.com/dominikl/omero-insight/releases/tag/v5.5.18 which were generated with the changes of this PR. All worked fine, except the Insight msi build. By default it wants to install itself in the Omero.importer directory. If you change the path during installation though it works fine too. |
Could you try with the msi using the release since I reckon it is a problem with the gradle build itself and not the switch to actions? |
Ok, this is really strange. I tried again and now like one would expect installation location is |
Thanks @dominikl |
Use the template file to use the correct gradle version
This PR fixes the GH actions
and add structure for releasing artifact
See https://github.com/jburel/omero-insight/releases/tag/v5.5.15
To test the generation via GHA on your fork