-
Notifications
You must be signed in to change notification settings - Fork 243
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
#2503 push SNAPSHOT Maven artifacts to github Packages #2525
base: master
Are you sure you want to change the base?
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.
LGTM, based on an initial quick glance... but a "core team" member and not just me should probably also rewiew this PR.
Hello @santosh-pingle, @ktarasenko Thanks |
@icrc-fdeniger FYI the artifact version number changes introduced here will cause build failures once you rebase this after #2533 is merged. The required fix (after you have rebased it, not right now) is as simple as changing those version numbers also in the (new, TBD) @MJ1998 @santosh-pingle @ktarasenko merge this only after #2533. |
5531d56
to
bb56824
Compare
@vorburger @santosh-pingle @ktarasenko should be ok now |
LGTM! I say let's merge this... @santosh-pingle @ktarasenko @MJ1998 no objects, right? |
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.
LGTM but requesting @aditya-07 to approve.
Thanks for this PR @icrc-fdeniger. LGTM. About the version proposal we would wanna keep the alpha/beta naming conventions. Reason - Using alpha and beta suffixes aligns with established conventions in semantic versioning and open-source software development. So we would like to stick with it. |
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.
LGTM
@santosh-pingle any news on that 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.
Thansk @icrc-fdeniger for this contribution! 🙏
For some background on the maven-repository.zip
artifact, see this PR from @yigit:
#249. We also have some documentation and I just realised it's a bit messed up after our recent documentation migration to this page: https://google.github.io/android-fhir/contrib but for your information the original text is this:
Use unreleased GitHub build (for development/testing only)
Each GitHub build of the project also contains a maven repository. You can access unreleased features by downloading it and including it in your project. To acquire the maven repository, go the the actions page, and click on the build you want. In the build
artifacts
dropdown, you will find amaven-repository.zip
file. After downloading and extracting it to a local folder, you can add it to your gradle setup via:repositories { maven { url "file:///<path to the unzipped folder>" } }
These artifacts are versioned as a combination of the current version + buildid (e.g.
0.1.0-alpha01-build_123
). You can find the version in the zip file by checking the contents of it and update your build file to match that version.
OK, now onto github packages - with this change, do we always overwrite the SNAPSHOT? so if I have a PR in development, and I push a new commit it will overwrite the github packages?
Another thing we might need to be careful about is just any incurred cost - https://github.com/features/packages#pricing. With our repo under google organisation I'm hoping that we'll be covered by the google organisation quota/allowance. But I will investigate.
* [Engine](api/engine/1.0.1/index.html) | ||
* [Data Capture](api/data-capture/1.1.1/index.html) | ||
* [Workflow](api/workflow/0.1.0-alpha05/index.html) | ||
* [Knowledge](api/knowledge/0.1.0-alpha04/index.html) |
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 would break our existing doc. please revert this change.
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.
It seems to me the PR is doing 2 things: set up the snapshot artifacts on github and updating the version numbers. In the spirit of making things independent, can we revert the version number change?
Fixes #2503
Description
Publish Maven artifacts in GitHub Packages
Type
Feature
Checklist
Versions
common
module will be automatically deployed with the version0.1.0-alpha06-SNAPSHOT
in GitHub Packages and with0.1.0-alpha06
in the next release.Suggestion/question
why not removeing beta/alpha as versions like 0.X.Y are considered as development version ?
if a dev Team wants to use an alpha/beta version it can use a dependency to a SNAPSHOT version ( the "unstable version created from the last build").
it's also possible to specify the artifact of a build with by example: engine-1.0.1-20240423.083712-3