-
Notifications
You must be signed in to change notification settings - Fork 13
Add a PR backporter #203
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 a PR backporter #203
Conversation
…base branches when we merge PRs to target branches.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 suggestions.
Comments skipped due to low confidence (2)
devtools/backport:114
- The commit_hashes variable should be joined with a space, not assigned a string with a space. Use 'commit_hashes = " ".join(args.commit_hashes)' instead.
commit_hashes = "{}".join(args.commit_hashes)
devtools/backport:212
- The get_github_token function should catch specific exceptions and provide a clear error message. Use 'except FileNotFoundError:' instead.
except:
Not bad Copilot - apply suggestion of version strip and writing token into a file. Co-authored-by: Copilot <[email protected]>
Is there a reason we can just use an off the shelf solution for backport? https://github.com/tibdex/backport seems to be the most popular. |
💚 Build Succeeded
cc @mashhurs |
We have a common script in LS core for this purpose: https://github.com/elastic/logstash/blob/main/devtools/backport Do you have any concerns with the LS core devtool/backporter? Honestly, I haven't seen any pain points so far with LS core backporter. If we have, I would support migrating all LS core and other plugin backporters (if we have) altogether. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
My only concern apart from general maintainability issues using our own script vs consuming a popular one is copying around files that need updates in the body https://github.com/elastic/logstash-filter-elastic_integration/pull/203/files#diff-38840c5e92fffb039a64f7632661853ab0075df02cb3f3dc123f1de961936a27R100
I'm fine with just doing this for now while we evaluate benefits of using a pre built action :)
Ah from that perspective makes sense. Let me create a placeholder to push forward. For now I am not seeing its urgency or broken features, it is nice to have tech dept! |
@logstashmachine backport 8.x |
1 similar comment
@logstashmachine backport 8.x |
Description
This PR adds a PR backporter which we need to backport changes to designated base branches when we merge PRs to target branches.