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

ICU-22721 Prevent inconsistent ICU4J Maven deploys via CI #2970

Merged
merged 1 commit into from
Apr 22, 2024

Conversation

echeran
Copy link
Contributor

@echeran echeran commented Apr 18, 2024

Update the CI workflow that deploys Maven artifacts to Github for ICU4J so that:

  • We don't publish the icu4j artifact to Github for release (non-snapshot) versions. This preserves Maven Central as the single source of truth for icu4j artifacts for release versions
    • Note: we keep the current behavior for snapshot versions, which is to publish to Github
  • We automatically prevent any manual attempts to publish a release version artifact

Trial runs in my personal fork:

Checklist
  • Required: Issue filed: https://unicode-org.atlassian.net/browse/ICU-22721
  • Required: The PR title must be prefixed with a JIRA Issue number.
  • Required: The PR description must include the link to the Jira Issue, for example by completing the URL in the first checklist item
  • Required: Each commit message must be prefixed with a JIRA Issue number.
  • Issue accepted (done by Technical Committee after discussion)
  • Tests included, if applicable
  • API docs and/or User Guide docs changed or added, if applicable

pedberg-icu
pedberg-icu previously approved these changes Apr 18, 2024
Copy link
Contributor

@mihnita mihnita left a comment

Choose a reason for hiding this comment

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

Nitpicking on words.

  • "manual" => my understanding (from the email thread) is that this can also run automatically, in some cases. So it is not always "manual"?
  • "deploys" vs "deployments"

Thank you!
M.

Mihai

then echo "version-type=snapshot" >> "$GITHUB_OUTPUT"
else echo "version-type=release" >> "$GITHUB_OUTPUT"
fi
- name: Prevent manual deploys of full release
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
- name: Prevent manual deploys of full release
- name: Prevent deployments of release versions

- name: Prevent manual deploys of full release
if: github.event_name == 'workflow_dispatch' && steps.mvn-proj-version-type.outputs.version-type == 'release'
run: |
echo "Manual deploys of publishing artifacts should only be done for snapshot versions"
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
echo "Manual deploys of publishing artifacts should only be done for snapshot versions"
echo "Deployments of the icu4j artifacts should only be done for snapshot versions"

@jira-pull-request-webhook
Copy link

Notice: the branch changed across the force-push!

  • .github/workflows/maven.yaml is different

View Diff Across Force-Push

~ Your Friendly Jira-GitHub PR Checker Bot

@echeran
Copy link
Contributor Author

echeran commented Apr 19, 2024

Nitpicking on words.

  • "manual" => my understanding (from the email thread) is that this can also run automatically, in some cases. So it is not always "manual"?
  • "deploys" vs "deployments"

Thank you! M.

Mihai

I made a few wording changes, including the "deploy" -> "deployment" change.

I kept "manual" because that is used in a step that is an additional early exit check to provide a more explanatory warning to users of what is & isn't an appropriate usage of artifact deployment in relation to versions. This extra check explicitly addresses the scenario that triggered the whole email thread in the first place. If not for the above reasons, then yes, that particular step is not entirely necessary from a technical perspective because this PR down below in later steps already prevents publishing of icu4j for snapshot releases.

@echeran echeran force-pushed the ci-github-maven-publish branch from d0e3aff to 23e9cee Compare April 22, 2024 22:11
@jira-pull-request-webhook
Copy link

Notice: the branch changed across the force-push!

  • .github/workflows/maven.yaml is different

View Diff Across Force-Push

~ Your Friendly Jira-GitHub PR Checker Bot

@echeran echeran merged commit 0312308 into unicode-org:main Apr 22, 2024
81 checks passed
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.

4 participants