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

Use version from PR title when edited by end user #26

Conversation

dgellow
Copy link
Member

@dgellow dgellow commented Sep 4, 2023

To determine if the user edited the version from the PR title release-please does the following:

  1. before building a release PR object, check if a PR already without a label exists for the release branch
  2. if the PR has a label autorelease: custom version, use the version from the title and stop here
  3. fetch the manifest from the release branch, get the version number for the relevant component
  4. compare the version number with the PR title version number
  5. if they don't match, we assume the end user edited the PR title version and use it
  6. add a label autorelease: custom version to the PR
  7. write a comment to the PR "The release version will not be automatically updated as it has been manually edited. If you want to use the calculated version remove the label autorelease: custom version"

For my tests I am using a github action like this one:

name: Handle release PR title edits
on:
  pull_request:
    types:
      - edited
      - unlabeled

jobs:
  update_pr_content:
    name: Update pull request content
    if: |
      (github.event.type == 'edited' && github.event.changes.title.from != github.event.pull_request.title) ||
      (github.event.type == 'unlabeled' && github.event.label.name == 'autorelease: custom version') &&
      github.event.pull_request.state == 'open' &&
      github.event.sender.login != 'stainless-bot' &&
      github.repository == 'stainless-sdks/sink-java-public'
    runs-on: ubuntu-latest
    steps:
      - uses: actions/checkout@v3
      - uses: stainless-api/trigger-release-please@v1
        with:
          repo: ${{ github.event.repository.full_name }}
          stainless-api-key: ${{ secrets.STAINLESS_API_KEY }}

You can find an example of a release with a custom label here, and the release pull request here.

@dgellow
Copy link
Member Author

dgellow commented Sep 4, 2023

The feature works, at least for the base strategy. Before merging I will add some unit tests to be sure all strategies have the same behaviour.

@dgellow
Copy link
Member Author

dgellow commented Sep 4, 2023

Just thought about something simplifying the feature, instead of using a label we can check the current version from the release-please manifest from the release branch (before updating it of course): if it does not match the PR title, then the PR title has been edited.

Edit: PR description updated to match this logic

@dgellow dgellow marked this pull request as draft September 4, 2023 19:33
@dgellow
Copy link
Member Author

dgellow commented Sep 4, 2023

Ok, I think that's way better. I need to test and write tests tomorrow.

@RobertCraigie
Copy link
Member

if it does not match the PR title, then the PR title has been edited.

How does this work if the PR title has not been edited and another commit is pushed that would further increase the version number? e.g.

  • PR opened with a single patch commit, results in v3.0.1
  • The user edits the version to v3.0.1-beta
  • A feat commit is pushed, would result in v3.1.0
    • At this time how do we know if v3.0.1 was set by the user or us?

Separately, is there any handling for the case where a commit is pushed after the user has edited the title? I think it could be quite nice to have a comment that the bot automatically updates that says something along the lines of:

Edited Notification

The release version will not be automatically updated as it has been manually edited.

Generated release version:

v3.4.5

@dgellow
Copy link
Member Author

dgellow commented Sep 5, 2023

How does this work if the PR title has not been edited and another commit is pushed that would further increase the version number

This wouldn't be an issue, at the time we compare the version from the PR title to the version in the manifest the version bump didn't occur yet.

You can see it here:

https://github.com/stainless-api/release-please/pull/26/files#diff-580375fac796e3ac9eaac08ce5f7fbeffe915e755615d04103fa2899fcd437d5R303-R350

  • PR opened with a single patch commit, results in v3.0.1
  • The user edits the version to v3.0.1-beta
  • A feat commit is pushed, would result in v3.1.0
    At this time how do we know if v3.0.1 was set by the user or us?

Separately, is there any handling for the case where a commit is pushed after the user has edited the title

Your example here seems to match the situation when another commit is pushed after an edit. That's something I thought about yesterday evening. The best solution I found is to add a label autorelease: custom version the first time we see a difference between the PR title version and the PR manifest version. If the label is set, just use the version from the title without any other check. If the user wants to go back to the calculated version, just remove the label?

Regarding the bot comment, that's a great idea, we can have release-please write a comment to the PR in addition to setting the label autorelease: custom version.

@dgellow dgellow changed the title Use version from PR title when labelled as user edited Use version from PR title when edited by end ser Sep 5, 2023
@dgellow dgellow changed the title Use version from PR title when edited by end ser Use version from PR title when edited by end user Sep 5, 2023
@dgellow
Copy link
Member Author

dgellow commented Sep 5, 2023

I added PR comments for warnings and issues, and to communicate to the user how to the end user how they can change back to generated versions.

The different states can be seen on this PR:

image

@dgellow dgellow marked this pull request as ready for review September 5, 2023 14:11

:rotating_light: This Pull Request has the \`${DEFAULT_CUSTOM_VERSION_LABEL}\` label but the version number cannot be found in the title. Instead the generated version \`${newVersion}\` will be used.

If you want to use a custom version be sure to use the semver format.

Choose a reason for hiding this comment

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

semver isn't something most people know what it means. Either explain or link to description?

Copy link
Member Author

Choose a reason for hiding this comment

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

Fair point. I actually started with a link to the semver spec but they aren't really user friendly. Would a link to something like this cheat sheet be more helpful? https://devhints.io/semver

Copy link
Member Author

Choose a reason for hiding this comment

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

Another option would be wikipedia? Not as user friendly: https://en.wikipedia.org/wiki/Software_versioning#Semantic_versioning

Choose a reason for hiding this comment

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

Cheat sheet link is good enough. Once it's clear what it means, users can investigate further, but the cheat sheet covers 99% (that's a very accurate madeup statistic)

@dgellow
Copy link
Member Author

dgellow commented Sep 5, 2023

Ok, now we have a test for this feature covering all the release type we are using: simple, node, python, and go 🐱.

When changing things related to the base strategy or from src/manifest.ts, I'm always afraid I'm breaking something for one of the plugin or strategy. And it's really time consuming if they have to be tested manually against actual repos.

…dited'

That makes it possible for an end user to control the version number number
without having to rely on a commit with the footnote RELEASE-AS.

The label can be easily added by a GitHub Action like this one:

```yaml
name: Handle release PR edits
on:
  pull_request:
    types:
      - edited

jobs:
  update_pr_content:
    name: Update pull request content
    if: |
      github.event.pull_request.state == 'open' &&
      github.event.changes.title.from != github.event.pull_request.title &&
      github.event.sender.login != 'stainless-bot' # this is important to avoid infinite loops
    runs-on: ubuntu-latest
    steps:
      - uses: actions/checkout@v3

      - uses: actions/github-script@v6
        with:
          script: |
            github.rest.issues.addLabels({
              issue_number: context.issue.number,
              owner: context.repo.owner,
              repo: context.repo.repo,
              labels: ['autorelease: user edited'] # add the label
            })

      - name: Then here run release-please
      - run: ...
```
@dgellow dgellow force-pushed the sam/sta-2086-customer-should-have-way-to-control-a-release-version-number branch from eced66d to 6d150d2 Compare September 6, 2023 15:29
@dgellow dgellow merged commit 99a4b0b into main Sep 6, 2023
7 checks passed
@dgellow dgellow deleted the sam/sta-2086-customer-should-have-way-to-control-a-release-version-number branch September 6, 2023 15:33
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.

3 participants