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

feat: support wait for maven central #133

Merged
merged 8 commits into from
Sep 24, 2024
Merged

Conversation

v1v
Copy link
Member

@v1v v1v commented Sep 23, 2024

closes #132

What

Support to query the maven central if wait-for-maven-central is set to true by default is not the case.

@v1v v1v added the changelog:enhancement When you enhance an existing feature label Sep 23, 2024
@v1v v1v requested review from SylvainJuge and a team September 23, 2024 13:34
| `artifact-id` | Maven artifact-ID of the artifact | `true` | ` ` |
| `group-id` | Maven group-ID of the artifact | `true` | ` ` |
| `version` | Version of the artifact to wait for | `true` | ` ` |
| `wait-for-maven-central` | Whether to wait for the artifact to be available in the maven central repository. | `false` | `false` |
Copy link
Member

Choose a reason for hiding this comment

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

maybe add as footnote that when set to false, only the sonatype repository is checked, which is usually fine for snapshots, but for releases checking maven central might be preferable as the sonatype proxy repo might not reflect actual maven central state.

Copy link
Member Author

Choose a reason for hiding this comment

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

Sure thing

For the record, I decided to use this approach to avoid a breaking change since it's a change of behaviour

Copy link
Member Author

Choose a reason for hiding this comment

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

what do you think with the new changes? 61f4cc5

I added a longer description and also a new flag to support whether to validate sonatype or maven or both.

Copy link
Member

Choose a reason for hiding this comment

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

Looks good to me, adding a sentence to explain when to change the default for wait-for-maven-central might be welcome:

  • with default value, we just wait for the publication to complete on sonatype, but the actual availability in maven central might not be ready yet.
  • with true we wait for the artifact to be published in maven central, this should be used when building other artifacts by downloading from maven central, which is for example quite common for docker images.

Copy link
Member Author

Choose a reason for hiding this comment

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

Copy link
Member

@reakaleek reakaleek left a comment

Choose a reason for hiding this comment

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

LGTM

Nit: Adding wait-for to the attribute name looks a bit redundant to me.

the action name already implies we are waiting for something.. but just thinking out loud here.

not a blocker.

maven/await-artifact/README.md Outdated Show resolved Hide resolved
maven/await-artifact/action.yml Outdated Show resolved Hide resolved
maven/await-artifact/action.yml Outdated Show resolved Hide resolved
maven/await-artifact/action.yml Outdated Show resolved Hide resolved
@v1v
Copy link
Member Author

v1v commented Sep 24, 2024

Nit: Adding wait-for to the attribute name looks a bit redundant to me.

I agree , see f3969cb

@v1v v1v enabled auto-merge (squash) September 24, 2024 12:29
@v1v v1v merged commit afe424c into elastic:main Sep 24, 2024
7 checks passed
v1v added a commit to v1v/oblt-actions that referenced this pull request Oct 7, 2024
…ibana

* upstream/main: (51 commits)
  deps: Bump oblt-cli version to 7.6.2 (elastic#139)
  feat: undeploy-my-kibana (elastic#140)
  build(deps): bump the github-actions group across 2 directories with 2 updates (elastic#141)
  build(deps): bump the github-actions group across 6 directories with 1 update (elastic#138)
  chore: deps(oblt-cli): Bump oblt-cli version to 7.5.24 (elastic#137)
  feat: support wait for maven central (elastic#133)
  feat: migrate is-member-elastic-org (elastic#135)
  deps: Bump oblt-cli version to 7.5.22 (elastic#131)
  deps(updatecli): bump all policies (elastic#130)
  ci: use GitHub app for ephemeral tokens (elastic#129)
  Deprecate the `project-id` input in `google/auth` action. (elastic#124)
  deps(updatecli): bump all policies (elastic#122)
  chore: deps(oblt-cli): Bump oblt-cli version to 7.5.21 (elastic#121)
  build(deps): bump the github-actions group across 11 directories with 4 updates (elastic#125)
  github-action: use ephemeral tokens with the required permissions (elastic#113)
  feat(github): validate-comment (elastic#120)
  feat(pre-commit): migrate from apm-pipeline-library (elastic#119)
  deps(updatecli): bump all policies (elastic#117)
  feat(await-maven-artifact): migrate from https://github.com/elastic/apm-pipeline-library (elastic#118)
  Add `test-report` action (elastic#114)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
changelog:enhancement When you enhance an existing feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

await-artifact only checks sonatype repo, not maven central
3 participants