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

ci(changelog): list the tag in the PR title #13234

Merged
merged 1 commit into from
Jul 1, 2024

Conversation

agilgur5
Copy link

Motivation

Make the automated changelog PRs have distinct titles for easy at-a-glance readability

Modifications

  • add the tag in the PR title and commit subject for the changelog.yaml GHA Workflow

  • also "updated" -> "update" for active voice/present tense

Verification

- and commit subject
  - use GHA [`github` context](https://docs.github.com/en/actions/learn-github-actions/contexts#github-context) for the ref name

- also "updated" -> "update" for active voice/present tense

Signed-off-by: Anton Gilgur <[email protected]>
@agilgur5 agilgur5 added area/docs Incorrect, missing, or mistakes in docs area/build Build or GithubAction/CI issues prioritized-review For members of the Sustainability Effort labels Jun 22, 2024
Copy link
Member

@terrytangyuan terrytangyuan left a comment

Choose a reason for hiding this comment

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

Could you merge multiple PRs into one?

@agilgur5
Copy link
Author

agilgur5 commented Jun 24, 2024

This and #13235 are independent of each other, so that would not be best practice (they were, in fact, intentionally split apart), as said several times previously by multiple contributors.

@agilgur5 agilgur5 requested a review from terrytangyuan June 24, 2024 00:10
title: 'docs: updated CHANGELOG.md'
commit-message: 'docs: updated CHANGELOG.md'
title: 'docs: update CHANGELOG.md for ${{ github.ref_name }}'
commit-message: 'docs: update CHANGELOG.md for ${{ github.ref_name }}'
Copy link
Contributor

Choose a reason for hiding this comment

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

I guess there's no good way to test this, huh? In that case, I just wanted to make sure there was no risk of ${{ github.ref_name }} not getting evaluated or not getting evaluated correctly, but I do see it listed here as "The short ref name of the branch or tag that triggered the workflow run. This value matches the branch or tag name shown on GitHub." which in this case should be the tag. So, I think I'm good.

Copy link
Author

@agilgur5 agilgur5 Jul 1, 2024

Choose a reason for hiding this comment

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

Other than making a fork and triggering actions (which is very tedious) unfortunately not.

act is the closest equivalent for local running, and it doesn't quite support everything either; not sure if it would work for this one. There is no official GItHub supported way otherwise 😕

And yes that's one of the reasons I linked the docs -- so that a reviewer could at least syntactically check

@agilgur5
Copy link
Author

agilgur5 commented Jul 1, 2024

@terrytangyuan this is blocked on your request changes

@terrytangyuan terrytangyuan merged commit 11ad50a into argoproj:main Jul 1, 2024
16 checks passed
@agilgur5 agilgur5 deleted the ci-changelog-pr-descriptive branch July 1, 2024 20:32
@agilgur5 agilgur5 added this to the v3.5.x patches milestone Jul 30, 2024
agilgur5 added a commit that referenced this pull request Jul 30, 2024
@agilgur5
Copy link
Author

agilgur5 commented Aug 1, 2024

Can confirm this worked for #13423 . Only thing is that, per #13414 (comment), this PR must apparently be cherry-picked into the release branch for it to take effect from a push of a tag off that branch.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/build Build or GithubAction/CI issues area/docs Incorrect, missing, or mistakes in docs prioritized-review For members of the Sustainability Effort
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants