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

Add support for dynamic targetPRLabels #391

Closed
jumaffre opened this issue Jun 6, 2022 · 7 comments · Fixed by #400
Closed

Add support for dynamic targetPRLabels #391

jumaffre opened this issue Jun 6, 2022 · 7 comments · Fixed by #400

Comments

@jumaffre
Copy link

jumaffre commented Jun 6, 2022

As far as I can see from the documentation and examples out there, the specified targetPRLabels is always static (typically set to ["backport"]). However, in some cases, it would be useful to be able to specify a dynamic label to add to the backport PR, e.g. to include the major version of the branch this PR is backported to.

For example, based on the existing branchLabelMapping, targetPRLabels could become: ["$1-backport"].

Please let me know if this use case makes sense and whether backport already supports such functionality. We've already been using this convention when manually backporting PRs to release branches and would like to continue doing so when automatising this process.

@sorenlouv
Copy link
Owner

This is a great idea. Using the regex placeholder ($1) is an interesting idea and it could definitely work. I can see this being a little confusing to users since targetPRLabels is not directly related to branchLabelMapping.

I've been thinking about adding support for a js-based config file (in addition to the current json-based one).

// .backportrc.js
module.exports = {
  targetPRLabels: (sourceCommit, targetBranch, sourcePRLabel) => {
    const match = sourceLabel.match(/(.*)-todo$/)?.[1];
    if (match) {
      return [match]
    }
  }
}

Do you have any preferences?

@jumaffre
Copy link
Author

jumaffre commented Jun 8, 2022

I agree that using $1 in targetPRLabels is far removed from branchLabelMapping. On the other hand, we don't have a use case (yet?) for using a custom JS script, but that sounds like the best option in this case as it's extensible and opt-in only.

@sorenlouv
Copy link
Owner

sorenlouv commented Jun 13, 2022

fyi: I've created #400 to support dynamic target labels like you suggested

@sorenlouv
Copy link
Owner

sorenlouv commented Jun 15, 2022

@jumaffre Support for dynamic targetPRLabels was added in #400 and released in v8.7.0.

I haven't bumped the github action yet. If you want to try it out, please specify main branch as version in the workflow file (/.github/workflows/backport.yml):

uses: sqren/backport-github-action@main

Please let me know if it works as expected. if it does I'll release the Github action

@jumaffre
Copy link
Author

@sqren This is working as expected. Thank you very much for sorting this out so quickly.

Separately to this, the following error occurred when I tried this new feature and tagged the same PR with two separate tags: https://github.com/microsoft/CCF/runs/7029861991?check_suite_focus=true. Could this be because our flow is a little different (we checkout the repository explicitly to avoid merge conflicts on a well-known file)?

@sorenlouv
Copy link
Owner

Separately to this, the following error occurred when I tried this new feature and tagged the same PR with two separate tags: https://github.com/microsoft/CCF/runs/7029861991?check_suite_focus=true. Could this be because our flow is a little different (we checkout the repository explicitly to avoid merge conflicts on a well-known file)?

Ouch, that looks bad. I don't know why multiple processes would run simultaneously and cause this. I'll try to see if I can reproduce.

@sorenlouv
Copy link
Owner

@jumaffre I haven't been able to repro even when using your workflow file. Does the problem happen consistently for you or intermittently?

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 a pull request may close this issue.

2 participants