-
Notifications
You must be signed in to change notification settings - Fork 303
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
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
…deploy-repo # Conflicts: # buildSrc/src/main/kotlin/Releases.kt
@santosh-pingle any news on that PR ? |
actually one last thing before merging - i feel we're really publishing a snapshot of the HEAD, instead of any particular versions... I couldn't find a way to do this but when we're publishing github packages can we just use the build id or something that is not tied to our version number for public releases? |
I guess this is better because someone might be using a particular snapshot in their dependency and could upgrade it to the latest build snapshot which is available. On the other hand, using version will overwrite the snapshot and the the developer may get confused that even though they are using the latest snapshot available they are not getting all the features added. |
Head branch was pushed to by a user without write access
@jingtang10 @vorburger documentation added. |
Sorry forgot to reply this earlier; yes we do: https://central.sonatype.com/search?q=com.google.fhir.gateway @icrc-fdeniger I have not read all the discussions on this PR but IIUC you want to use |
Thanks @bashir2 for your answer. For OpenMRS ( given as an example but true for all projects :)) there are 2 main use cases:
For the second point, for sure developpers can check-out/build the source locally but:
IMHO, deploying |
Maven Central is... erm, "evolving" ("cough"), and I think they won't support SNAPSHOT repos in the future. Just FYI, vorburger/ch.vorburger.exec#212 (unrelated, just FYI). |
hi @vorburger - can you pls add more details on maven central's support for snapshot? |
My advice is to stay clear of Maven Central, for SNAPSHOT. I advise to either use GitHub packages (with some auto clean), or simply a SNAPSHOT repo on the docs site. |
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.
tweaked a few things in the doc.
thanks so much @icrc-fdeniger for this! excited for this change!
thanks this change will help us :) |
No I've enabled auto merge so hopefully it'll go in once tests pass |
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