-
Notifications
You must be signed in to change notification settings - Fork 101
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
remove slice2java #6325
remove slice2java #6325
Conversation
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.
Although the build passes without issue with this PR included, both in the GitHub Actions context as well as locally (without slice2java installed), the server failed to start - see https://merge-ci.openmicroscopy.org/jenkins/job/OMERO-server/1234/
Looking quickly at the logs, the services did not get started. Looking at a few configuration files, il looks like a few variables did not get subsituted e.g. in etc/omero.properties
:
omero.version=5.6.3-242-d748290-ice${versions.ice_lib}-b1224
Since we are using only one ice version, that could be removed. |
Back to green https://merge-ci.openmicroscopy.org/jenkins/job/OMERO-server/1242/ with this PR included |
OMERO.script integration tests (Python + Java) have been failing for the last two days - see https://merge-ci.openmicroscopy.org/jenkins/job/OMERO-test-integration/1137/. This is possibly related to these changes |
https://merge-ci.openmicroscopy.org/jenkins/job/OMERO-test-integration/1144/ is green with this PR included |
In general, 👍 for getting rid of the unnecessary The biggest change looking at the nightly artifacts is the removal of the This raises essentially the same question as in #6318 (comment). Do we have an authoritative list of all the downstream locations that would be impacted by the change and will need to be reviewed? And do you have a feeling of the cost/time of updating these post server release? |
Briefly:
|
|
@joshmoore I think we can also drop |
The last 2 commits remove:
the build remains green |
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.
Thanks, the latest version of this PR feels like the minimal set of changes allowing to drop the unnecessary Ice requirement at the level of this repository without modifying artifact names and downstream consumptions.
No objection to merging from my side
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.
Also no objections. I imagine eventually, we strip out the rest of strip2cpp, etc. but happy to wait until we drop ant & ivy 😄
Remove slice2java.
Check that the build is green