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

Actions on PR and tag #3626

Closed
wants to merge 20 commits into from
Closed

Actions on PR and tag #3626

wants to merge 20 commits into from

Conversation

jburel
Copy link
Member

@jburel jburel commented Oct 27, 2020

Add support for actions.
Build on windows and ubuntu on pull_request
When a tag is pushed, the artifacts will be uploaded to the GitHub tag (similar to downloads.o.org)
I have not removed the hard-coded version for this round
Token will need to set on the repo in order to push the tag

I have not removed the travis or appeyvor files
I have tested the workflow using on my account see https://github.com/jburel/bioformats/releases/tag/v6.6.0-m3 for example

cc @dgault

@manics
Copy link
Member

manics commented Oct 29, 2020

@jburel
Copy link
Member Author

jburel commented Nov 2, 2020

@manics I have played with both of them, yes but opted for the solution put in place due to the steps required like creation of checksum etc.

@sbesson
Copy link
Member

sbesson commented Nov 5, 2020

A few comments:

  • 👍 for the Python 3 upgrade and the UpgradeChecker deprecation
  • re Actions on PR and tag #3626 (comment), I am definitely in favor of using upstream organizational roles whenever possible. A limitation is that the GitHub actions syntax does not support for loops so I assume it would mean duplicating the action for each asset including checksums. Mid-term I would hope Add the ability to use wildcards for uploading files. actions/upload-release-asset#60 will allow us to use actions/upload-release-asset with a wildcard.
  • most of the Travis CI workflow is now converted with the exception of the artifact deployment to Artifactory -

    bioformats/.travis.yml

    Lines 38 to 46 in c14984f

    deploy:
    provider: script
    script: "cp .travis.settings.xml $HOME/.m2/settings.xml && mvn deploy"
    skip_cleanup: true
    on:
    jdk: openjdk11
    repo: ome/bioformats
    branch: develop
    tags: false
    . Is this in scope of this work
  • the current workflow combines Ant and Maven as matrix components. An alternate representation could be to split them into two separate workflows: ant.xml and maven.xml. The main benefit of this approach is to be able to re-use the latter as the Maven commands are very generic. This can also be reviewed as a follow-up with other Maven-based Java libraries in mind.

@jburel
Copy link
Member Author

jburel commented Nov 5, 2020

I think:

  • I will remove the "release" file since we may not want to push all artifacts to GH for next release
  • split into ant and maven as suggested above

since we push over 10 artifacts and not having support for loop in actions, using upstream actions was making the file extremely long, and this chosen approach was not breaking if for example we decide to remove a jar. As indicated I will remove it for now and we can revisit when the deployment is in place.

@jburel
Copy link
Member Author

jburel commented Nov 16, 2020

closing this PR see #3635 for the build part of this work

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.

3 participants