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

Refactor release process in order to include changelog in release files #1246

Merged
merged 55 commits into from
May 17, 2024

Conversation

chrysle
Copy link
Contributor

@chrysle chrysle commented Feb 6, 2024

  • I have added a news fragment under changelog.d/ (if the patch affects the end users)

Summary of changes

This ought to correctly include the changelog in the docs on release, after PR merge.

Test plan

Tested by running

# command(s) to exercise these changes

@Gitznik
Copy link
Contributor

Gitznik commented Feb 6, 2024

I think this is an okay quick fix (potentially for the next release so we're not blocked by this?).

What I don't like about this is that the release artifacts in github releases would not contain the updated changelog. Not a huge deal I guess, but feels wrong.

@chrysle
Copy link
Contributor Author

chrysle commented Feb 7, 2024

What I don't like about this is that the release artifacts in github releases would not contain the updated changelog. Not a huge deal I guess, but feels wrong.

I understand. What alternative do you suggest? My idea would be to transfer the release-related jobs from the tests workflow to a new release workflow with a workflow_dispatch event, then configuring the "Bump changelog on release" job to actually create the release, causing the other jobs to run. We would end up running that workflow manually to release.

I could rework the PR towards that, WDYT?

@Gitznik
Copy link
Contributor

Gitznik commented Feb 7, 2024

I understand. What alternative do you suggest? My idea would be to transfer the release-related jobs from the tests workflow to a new release workflow with a workflow_dispatch event, then configuring the "Bump changelog on release" job to actually create the release, causing the other jobs to run. We would end up running that workflow manually to release.

I could rework the PR towards that, WDYT?

Yes, that would be my suggestion as well. Should lead to the cleanest result overall. Just mentioning that @gaborbernat mentioned that this project tries to avoid this kind of workflow, but I don't see a good alternative.

@gaborbernat
Copy link
Contributor

Where would you specify the version to release in a workflow_dispatch?

@Gitznik
Copy link
Contributor

Gitznik commented Feb 7, 2024

Where would you specify the version to release in a workflow_dispatch?

From one of our deployment workflows at work:
image

@gaborbernat
Copy link
Contributor

Works for me 👍 remember to update https://github.com/pypa/pipx/blob/main/CONTRIBUTING.md#releasing-new-pipx-versions

@chrysle
Copy link
Contributor Author

chrysle commented Feb 11, 2024

There is one complication which requires a bit more discussion. If we want to stick with opening a pull request every release to update the changelog, how can the same workflow be configured to create a GH release after that pull request is merged?

@Gitznik
Copy link
Contributor

Gitznik commented Feb 11, 2024

There is one complication which requires a bit more discussion. If we want to stick with opening a pull request every release to update the changelog, how can the same workflow be configured to create a GH release after that pull request is merged?

I think release-please does it by giving the changelog PR a specific label and watching for that label for creating the release.

@Gitznik
Copy link
Contributor

Gitznik commented Feb 29, 2024

Will you work on this soon @chrysle? If not I'd suggest we create a release soon with the current setup. There's a pretty good amount of bug fixes and new features to be released now.

If we do one with the current setup, I can manually create a PR updating the docs, so they show up correctly in the docs after the release?

@chrysle
Copy link
Contributor Author

chrysle commented Mar 1, 2024

There's a pretty good amount of bug fixes and new features to be released now.

Agreed. I just discovered that release-please unfortunately won't help in our case, as it doesn't allow for disabling the automatic changelog generation (yet, see googleapis/release-please#2007). Thus I guess I'm going to extract the bump-changelog job into a separate workflow.

@chrysle chrysle force-pushed the stable-tagging branch 4 times, most recently from 411ec66 to 3f62f1f Compare March 10, 2024 14:52
@chrysle
Copy link
Contributor Author

chrysle commented Mar 10, 2024

Updated. See the result in chrysle@fff3448.

@Gitznik
Copy link
Contributor

Gitznik commented Mar 10, 2024

While your current solution works, what about giving this changelog PR a specific label and run the release via pipeline when a PR with that label is merged?

Label is a direct part of the pull_request object when using the pull_request trigger, and the version to release we can extract from the PR title, which is also in the pull_request object.

Removes one manual step from the pipeline.

@chrysle
Copy link
Contributor Author

chrysle commented Mar 10, 2024

I'll look into it, it could take a little bit of time, though.

@gaborbernat gaborbernat marked this pull request as draft March 25, 2024 17:37
@chrysle chrysle changed the title Introduce stable tags Refactor release process in order to include changelog in release files Apr 9, 2024
@chrysle
Copy link
Contributor Author

chrysle commented Apr 14, 2024

This isn't quite that easy, as it turns out.. After trial and error for a while I finally managed to get my workflow to trigger on
a merged pull request with a specific label (see the successful run), but just as it creates the release, the tests workflow is already running on the push event, and the pushing of the release tag is somehow swallowed, because of which the CD isn't triggered. Any suggestions?

@chrysle
Copy link
Contributor Author

chrysle commented Apr 24, 2024

@henryiii May I ask for some advice? The current state is at https://github.com/chrysle/pipx/tree/test-cd.

@chrysle
Copy link
Contributor Author

chrysle commented May 2, 2024

@henryiii Pinging you on this as requested.

@Gitznik
Copy link
Contributor

Gitznik commented May 14, 2024

This isn't quite that easy, as it turns out.. After trial and error for a while I finally managed to get my workflow to trigger on a merged pull request with a specific label (see the successful run), but just as it creates the release, the tests workflow is already running on the push event, and the pushing of the release tag is somehow swallowed, because of which the CD isn't triggered. Any suggestions?

Just seeing this now. That sounds quite tricky indeed. As far as I'm concerned, unless someone that is really into github workflows wants to pick this up, I'm fine with having a 2 step release. Manually creating the release after merging the changelog changes is easy to document and not that difficult or prone to error.

I'm sure your time is better spent somewhere else than this ;) Sorry I sent you on this goose hunt.

@huxuan
Copy link
Member

huxuan commented May 14, 2024

IIUC, those release related jobs/steps should belong to a different workflow, which maybe release.yml. The workflow is triggerred when a pull request merged with specific label. It might be something like

name: Release

on:
  pull_request:
    types: [closed]

jobs:
  create-release:
    if: >-
        github.event_name == 'pull_request'
        && github.event.pull_request.merged == true
        && github.event.pull_request.labels.name == 'release-version'
    steps:
      - ...
      - ...

@chrysle
Copy link
Contributor Author

chrysle commented May 14, 2024

IIUC, those release related jobs/steps should belong to a different workflow, which maybe release.yml

Yup, you're seeing an outdated version here, actual work happened at https://github.com/chrysle/pipx/tree/test-cd. But your comment inspired me to attempt moving all the release-related jobs into this workflow, and I finally managed, although I went half crazy (this is the last time I've done anything with GH Actions here, rest assured). Here the successful run and the release it created. If you think that's all in order, I'll merge the development branch; otherwise, I'll give up.

@huxuan
Copy link
Member

huxuan commented May 14, 2024

If you think that's all in order, I'll merge the development branch

IMO, it looks pretty good.

Previously, I just use a tag in semver to trigger the release process. This two steps release process is quite a good idea. I may also apply it in my personal projects. :-)

Gitznik
Gitznik previously approved these changes May 16, 2024
Copy link
Contributor

@Gitznik Gitznik left a comment

Choose a reason for hiding this comment

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

Great stuff

Copy link
Member

@huxuan huxuan left a comment

Choose a reason for hiding this comment

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

Just realize some concern about concurrency.

.github/workflows/release.yml Outdated Show resolved Hide resolved
.github/workflows/bump-changelog.yml Show resolved Hide resolved
@chrysle
Copy link
Contributor Author

chrysle commented May 17, 2024

Thanks for correcting my oversight! Updated.

Copy link
Member

@huxuan huxuan left a comment

Choose a reason for hiding this comment

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

Great job!

@gaborbernat gaborbernat merged commit 2970397 into pypa:main May 17, 2024
11 checks passed
DimitriPapadopoulos pushed a commit to DimitriPapadopoulos/pipx that referenced this pull request May 18, 2024
…es (pypa#1246)

* Extract `bump-changelog` action into separate workflow

* Create `create-release` job in `tests.yml`

* Bump changelog for 1.0.0 (pypa#2)

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>

* Specify pull request types that trigger CI

* Remove requirement for job

* Try to fix label detection

* Try out `pull_request_target` event

* Fix environment variable access syntax

* Try splitting out in separate workflow

* Use concurrency

* Create `create-release` job in `tests.yml`

* Specify pull request types that trigger CI

* Remove requirement for job

* Try to fix label detection

* Try out `pull_request_target` event

* Fix environment variable access syntax

* Try moving related jobs into release workflow

* Specify Python version in release workflow

* Amend for testing

* Retrieve artifact from previous workflow run

* Create release before uploading asset (makes sense)

* Specify tag name

* Don't run in parallel *yawn*

* Get that blasted tag

* Permissions

* Cleanup, docs

* Extract `bump-changelog` action into separate workflow

* Create `create-release` job in `tests.yml`

* Bump changelog for 1.0.0 (pypa#2)

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>

* Specify pull request types that trigger CI

* Remove requirement for job

* Try to fix label detection

* Try out `pull_request_target` event

* Fix environment variable access syntax

* Try splitting out in separate workflow

* Use concurrency

* Create `create-release` job in `tests.yml`

* Specify pull request types that trigger CI

* Remove requirement for job

* Try to fix label detection

* Try out `pull_request_target` event

* Fix environment variable access syntax

* Try moving related jobs into release workflow

* Specify Python version in release workflow

* Amend for testing

* Retrieve artifact from previous workflow run

* Create release before uploading asset (makes sense)

* Specify tag name

* Don't run in parallel *yawn*

* Get that blasted tag

* Permissions

* Cleanup, docs

* Better sentence structure

Co-authored-by: Robert <[email protected]>

* Use concurrency in `bump-changelog.yml`

And fix concurrency group definition in
`release.yml`.

---------

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Co-authored-by: Robert <[email protected]>
@chrysle chrysle deleted the stable-tagging branch May 21, 2024 05:58
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