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

Review release process docs for PR-based releases #2499

Closed
stefan6419846 opened this issue Mar 3, 2024 · 8 comments · Fixed by #2554
Closed

Review release process docs for PR-based releases #2499

stefan6419846 opened this issue Mar 3, 2024 · 8 comments · Fixed by #2554
Assignees
Labels

Comments

@stefan6419846
Copy link
Collaborator

stefan6419846 commented Mar 3, 2024

https://pypdf.readthedocs.io/en/stable/dev/releasing.html#how-is-it-done cannot be applied directly when not having direct write access to the main branch. The following comments document what I did and observed:

  1. Run python make_release.py

  2. Adapt the three generated files. For version 4.1.0, DEV: Fix changelog generator regarding whitespace and handling of "Other" group #2492 had to be mitigated.

  3. Create new branch: git checkout -b release_4.1.0

  4. Commit the changes: git commit --file RELEASE_COMMIT_MSG.md (Do not use the -e parameter here - this will open the editor and might strip the headings due to the lines starting with # being interpreted as comments.)

  5. Verify the commit message: git log

  6. Create PR and ask for review

  7. The PR title check is failing - needs some adjustments

  8. Merge the PR, ensuring that the title only is REL: 4.1.0 and the body contains the correct message.

  9. git checkout main && git pull

  10. Create the tag: git tag 4.1.0 --file RELEASE_TAG_MSG.md (No -s due to not signing it.)

  11. Verify the tag message: git show 4.1.0

    • In my case, I needed git config core.commentChar ";" before creating the tag, as otherwise the # lines would disappear again ...
    • Delete a wrong tag: git tag -d 4.1.0
  12. Push the tag: git push --tags - Permissions not sufficient

  13. CI should do the remaining steps of creating the release and uploading the release to PyPI

@stefan6419846 stefan6419846 self-assigned this Mar 3, 2024
@stefan6419846
Copy link
Collaborator Author

stefan6419846 commented Mar 3, 2024

@MartinThoma Apparently, we are still missing the permission to create tags (at least through SSH, pushing to custom branches on the repository works correctly):

stefan@stefan2023:~/tmp/pypdf/pypdf_upstream$ LANG=C git push origin 4.1.0 
Enumerating objects: 1, done.
Counting objects: 100% (1/1), done.
Writing objects: 100% (1/1), 916 bytes | 916.00 KiB/s, done.
Total 1 (delta 0), reused 0 (delta 0), pack-reused 0
remote: error: GH006: Protected tag update failed for refs/tags/4.1.0.
remote: error: You're not authorized to create a tag
To github.com:py-pdf/pypdf.git
 ! [remote rejected] 4.1.0 -> 4.1.0 (protected tag hook declined)
error: failed to push some refs to 'github.com:py-pdf/pypdf.git'

@pubpub-zz For your info.

@MartinThoma
Copy link
Member

I think we should change the process a bit. I think we can and should create release tags automatically from the release commit. That makes it easier, less prone to inconsistencies, and multiple contributors can decide on making a release together.

@stefan6419846
Copy link
Collaborator Author

I am generally open to improving this part of the process if it means that it simplifies releasing. Generally, I am planning to rework the releasing docs anyway to reflect the PR-based process and document some of the pitfalls I experienced when doing the process the first time.

@MartinThoma
Copy link
Member

MartinThoma commented Mar 3, 2024

The Trigger

I just saw this and e can use startsWith to check for REL.

So something like:

jobs:
  main:
    name: Build and test
    runs-on: ubuntu-latest
    if: "startsWith(github.event.head_commit.message, 'REL: ')"

The tag

We can create a git tag via https://github.com/marketplace/actions/github-tag

I'm not sure if it would also work to have just a shell command 🤔 I would prefer that - less dependencies meaning less that could break / be abused.

Putting it together

name: Add a git tag for the release

on:
  push:
    branches:
      - main

jobs:
  main:
    name: tag the commit
    runs-on: ubuntu-latest
    if: startsWith(github.event.head_commit.message, 'REL: ')

    steps:
      - name: Checkout code
        uses: actions/checkout@v4

      - name: Extract version from commit message
        id: extract_version
        run: |
          VERSION=$(echo "${{ github.event.head_commit.message }}" | grep -oP '(?<=REL: )\d+\.\d+\.\d+')
          echo "::set-output name=version::$VERSION"

      - name: Create Git Tag
        run: |
          VERSION="${{ steps.extract_version.outputs.version }}"
          git tag "$VERSION" -m "${{ github.event.head_commit.message }}"
          git push --tags

@MartinThoma
Copy link
Member

I'm trying it here: https://github.com/MartinThoma/sample-repo

@MartinThoma
Copy link
Member

I think I found a way that works: https://github.com/MartinThoma/sample-repo/tags :-)

@MartinThoma
Copy link
Member

@stefan6419846 Now that #2500 is merged, do you think this issue can be closed?

@stefan6419846
Copy link
Collaborator Author

@MartinThoma We at least should fix (2) and (7) in my opinion, id est review #2492 and #2554. The remaining adjustments are part of #2553.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants