Skip to content
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

Deploy #3647

Merged
merged 8 commits into from
Dec 17, 2020
Merged

Deploy #3647

merged 8 commits into from
Dec 17, 2020

Conversation

jburel
Copy link
Member

@jburel jburel commented Dec 10, 2020

This PR deploys the snapshots to artifactory when a PR is merged into develop.

I have also renamed the ant workflow so it is easier to include it as a badge and so it matches the approach taken in other files.

secrets will need to be set for the deployment to work
cc @sbesson @dgault

Screenshot 2020-12-10 at 17 06 36

Edit:
The logic to push is as follow:

  • if a commit with version with snapshot is pushed to develop. Artifacts are pushed to ome.snapshots
  • if a commit with version without snapshot is pushed to develop. Artifacts are pushed to ome.staging
  • No deployment when a tag is push, we can deploy on tag but we need to add check to make sure we do not push twice i.e. develop without snapshot and tag

Copy link
Member

@sbesson sbesson left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Only concern is that the current setup is likely to fail for non-SNAPSHOT versions. On the other side, builds are currently hanging on Travis and this converts the remaining bit of Travis into a workflow and ideally should preserve the automatic build and deployment of the 6.6 Maven artifacts.

No objection to seeing this merged tomorrow in from my side. But we might need to be ready for some adjustments in the upcoming builds. Additionally, it might be worth testing with a 6.6.0-rc1 workflow to confirm everything works as expected with artifacts being built and uploaded to ome.staging.

.github/workflows/maven.yml Outdated Show resolved Hide resolved
.github/workflows/maven.yml Show resolved Hide resolved
@jburel
Copy link
Member Author

jburel commented Dec 11, 2020

With the last commit:
branch name develop on my fork, https://github.com/jburel/bioformats/actions/runs/414839434
Screenshot 2020-12-11 at 07 26 57

@jburel
Copy link
Member Author

jburel commented Dec 11, 2020

https://github.com/jburel/bioformats/actions/runs/414967940 This is a run testing rc1 push to develop
Screenshot 2020-12-11 at 09 05 53

Copy link
Member

@sbesson sbesson left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The changes faithfully converts the current Travis deployment set-up i.e. allowing to deploy the artifacts built on every commit to the develop branch either to ome.snapshots or to ome.staging.

Organization-wide CI_DEPLOY_USER/CI_DEPLOY_PASS secrets have been configured with a dedicated github-actions agent allowing the deployment to the OME artifactory. Merging to start testing the integration.

A few comments:

  • while the SNAPHSOT deployment should be thoroughly tested, we might want to pre-test the non-SNAPSHOT deployment ahead of the next release of Bio-Formats /cc @dgault
  • CI_DEPLOY_USER/CI_DEPLOY_PASS might need to be reviewed when we start deploying to Sonatype via GitHub actions
  • as mentioned by @jburel, the current deployment workflow expects a commit bumping the version to trigger a deployment. An alternative workflow used in bio-formats-example is to deploy on tags. In the case of this repository where every commit triggers a deployment, this would require either extra conditional logic to prevent two actions (one on push and one on tag) to try and deploy the same version to ome.staging.

@sbesson sbesson merged commit b838003 into ome:develop Dec 17, 2020
@sbesson
Copy link
Member

sbesson commented Dec 17, 2020

Screenshot 2020-12-17 at 12 24 40

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants