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

One-step release action #368

Open
wants to merge 29 commits into
base: master
Choose a base branch
from
Open

One-step release action #368

wants to merge 29 commits into from

Conversation

ajjackson
Copy link
Collaborator

Closes #326
Replaces #364
(Forgot that when branch name contains "release" tests get run twice. Should probably clean that up, but for now just use a different branch name...)

@ajjackson ajjackson mentioned this pull request Feb 9, 2025
@ajjackson ajjackson changed the title 326 rls action One-step release action Feb 9, 2025
Copy link
Contributor

github-actions bot commented Feb 9, 2025

Test Results

    18 files   -   2      18 suites   - 2   1h 3m 8s ⏱️ - 1m 49s
 1 083 tests ±  0   1 077 ✅ ±  0   6 💤 ±0  0 ❌ ±0 
10 387 runs  +531  10 333 ✅ +536  54 💤  - 5  0 ❌ ±0 

Results for commit c1f259b. ± Comparison against base commit 0554c1f.

♻️ This comment has been updated with latest results.

Copy link

codecov bot commented Feb 9, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 96.04%. Comparing base (0554c1f) to head (c1f259b).

Additional details and impacted files
@@           Coverage Diff           @@
##           master     #368   +/-   ##
=======================================
  Coverage   96.04%   96.04%           
=======================================
  Files          31       31           
  Lines        4219     4219           
  Branches      645      645           
=======================================
  Hits         4052     4052           
  Misses         95       95           
  Partials       72       72           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@ajjackson ajjackson force-pushed the 326-rls-action branch 7 times, most recently from be924f4 to 60365cf Compare February 10, 2025 16:57
@ajjackson
Copy link
Collaborator Author

https://github.com/pace-neutrons/Euphonic/actions/runs/13246775974
is the most realistic check I dare run for now: pypi push is commented out but everything else live. (Will need to pull the ensuing Github release...)

@ajjackson
Copy link
Collaborator Author

All looks pretty convincing amd release page has appeared. GH is also showing some scary deployment rocket logos, but as far as I can tell nothing actually went to PyPi.

I guess the next step is to uncomment the PyPI lines, merge this, and try it for real 🫣

@ajjackson ajjackson marked this pull request as ready for review February 10, 2025 19:00
@ajjackson
Copy link
Collaborator Author

ajjackson commented Feb 11, 2025

Taking a closer look at the create-landing-page workflow:

name: create-landing-page
on:
  push:
    branches:
      - master
    paths:
      - 'CITATION.cff'
  release:
    types: [published]

This is currently designed to fire off if you

  • push a new CITATION.cff to master, or
  • make a Release

But it didn't fire off when I tested the new release.yml action.
I think this is due to Github's restriction on actions-triggering-actions (which helps avoid infinite loops...) https://github.com/orgs/community/discussions/25281
This can be solved by adding a workflow-call trigger; in that case I would be tempted to remove the existing release trigger as we don't intend to release without calling release.yml.

Manual dispatch would not be especially useful:

  • The script will get the info from a tagged commit; as release commits are more-or-less immutable (as decided by PyPI!) it should always give the same result for a given version.
  • The "workaround" for correcting an existing release is to edit the rendered gh-pages branch directly.
  • While on that branch, it's pretty simple to run write_doi_landing_page.py if that's helpful.

There is also a question around how the "push" trigger interacts with release branches. After releasing we would typically merge the release branch to master and this could trigger another landing page build for the already-released version. This will update the "latest" page (as opposed to a specific version) found at https://pace-neutrons.github.io/Euphonic/. So that's probably working as intended, actually?

@ajjackson
Copy link
Collaborator Author

Ok, that last run successfully deployed a 1.4.1rc landing page (which I had better clean up...) but did not create a corresponding "latest" page, so "latest" still shows up as the previous release.

Perhaps for now we will leave that be and rely on the eventual merge to master to take care of it.

I think everything is done here; I'll clean up the garbage releases and uncomment the PyPI push lines.

Now this script can be used in the following ways:

- without "--replace" argument the new CHANGELOG is printed to stdout
- If there is Unreleased content and the requested tag already exists
  right after Unreleased, we assume that a failed release is being
  fixed. Content from Unreleased is moved into that section and appended
  to what is already there.
- If the tags match but there are no Unreleased changelog notes,
  nothing much should change. (Maybe some line breaks.)
  This might be the case if we are on a release branch and already
  manually updated the CHANGELOG.
- Otherwise, we create a new release section under Unreleased, move
  content from Unreleased into that section, and update the "compare"
  links.

While working on the parser I ran into a broken link and some
inconsistent spacing in the existing CHANGELOG.rst file.
This could be done in a _slightly_ more modular fashion but would add
a bit of complexity as well to pass around the file changes across
actions without making multiple commits in the history.

The nice thing about this setup is that bump_changelog.py doesn't mind
at all if you already sorted out the CHANGELOG by hand, so if there
are any difficult/edge cases they can be handled by using a release
branch and running the script locally before using the release action.
itertools.batched is from Python 3.12. Not a complete dealbreaker, but
makes life simpler as developers are likely running the script from a
Euphonic dev environment which is 3.10 with toolz available.
Change of plan: we _do_ need to push the version/tag commit in order
to have a sensible multi-job workflow. We do want that multi-job
workflow because versioning tweaks should happen on one OS but
building/testing needs to be matrixed.

But this should generally be done on a release branch, so if the
release fails it will be possible to clean up these commits/tags
without messing up the master branch history.

It seems a good idea to provide standalone build/test actions for
debugging. By separating the sdist and wheel builds they can be called
in parallel as part of overall release workflow.

A couple of details still to work out at this point:

- The OS matrix should be moved up to release.yml and passed as an
  option to build_wheels, so it is possible to troubleshoot one OS at a
  time.

- We may need to store the normalised release tag to environmnent and use
  in subsequent steps instead of inputs.ref. That way, small formatting
  changes like adding "v" will be carried forward correctly.

  https://docs.github.com/en/actions/sharing-automations/reusing-workflows#using-outputs-from-a-reusable-workflow
The workflow visualisation tends to put the "test sdist" step right on
top of the dependency line from build-wheels to release.

This is quite misleading as it looks like test sdist depends on
build-wheels and yet it can start running before build-wheels is done.

Perhaps if we swap the jobs they will be stacked in a different order
and it will look clearer?
We could streamline things a bit by moving the code directly into
release.yml, but for now let's keep options open.
This seems like overkill, we have already been quite careful to create
consistent version information. But it wouldn't hurt to have these
checks running for an automated release cycle before we consider
pruning them.
release.py is complaining that CHANGELOG is out of sync. How can that be?
Release script correctly picked up that the version number was written
to changelog without the leading "v": this is inconsistent!
This is a pretty confusing Actions behaviour: the github.event_name is
inherited from the calling workflow so create-landing-page thinks it
is a workflow_dispatch when called from release.yml
@ajjackson ajjackson requested a review from oerc0122 February 11, 2025 11:50
@ajjackson
Copy link
Collaborator Author

@oerc0122 I think I've run enough sanity checks that it works, but maybe you could have a glance at the overall logic?

Copy link
Collaborator

@oerc0122 oerc0122 left a comment

Choose a reason for hiding this comment

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

Looks good function-wise. Just some possible alternatives in the Python.

build_utils/bump_changelog.py Outdated Show resolved Hide resolved
build_utils/bump_changelog.py Outdated Show resolved Hide resolved
build_utils/bump_changelog.py Outdated Show resolved Hide resolved
build_utils/bump_changelog.py Outdated Show resolved Hide resolved
build_utils/bump_changelog.py Outdated Show resolved Hide resolved
build_utils/bump_changelog.py Outdated Show resolved Hide resolved
.github/workflows/build_sdist.yml Show resolved Hide resolved
ajjackson and others added 2 commits February 11, 2025 14:48
Co-authored-by: Jacob Wilkins <46597752+oerc0122@users.noreply.github.com>
ajjackson and others added 2 commits February 11, 2025 15:17
Co-authored-by: Jacob Wilkins <46597752+oerc0122@users.noreply.github.com>
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.

Create a release Workflow that integrates version-numbering and runs tests before pushing
2 participants