-
Notifications
You must be signed in to change notification settings - Fork 107
[mergebot] Set mergebot to be configured via config #5312
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
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
ZainRizvi
left a comment
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.
🚢
| import { CachedConfigTracker } from "./utils"; | ||
|
|
||
| function pytorchBot(app: Probot): void { | ||
| const cachedConfigTracker = new CachedConfigTracker(app); |
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.
any idea how long the cache is persisted?
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.
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.
It's not persisted at all, but repeated calls to config methods during execution of the command would not need to make new network calls to fetch the same info.
huydhn
left a comment
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.
LGTM! Pending your change on PT bot config
Fixes #ISSUE_NUMBER * Companion to pytorch/test-infra#5312 * See the above for details + possible risks * Without the above PR, this should have no effects Pull Request resolved: #128840 Approved by: https://github.com/huydhn
| import { CachedConfigTracker } from "./utils"; | ||
|
|
||
| function pytorchBot(app: Probot): void { | ||
| const cachedConfigTracker = new CachedConfigTracker(app); |
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.
It's not persisted at all, but repeated calls to config methods during execution of the command would not need to make new network calls to fetch the same info.
Tested locally using --dry-run Apparently there is precedent for just copying the files over (see executorch PR huy linked below), so this copies over trymerge + dependencies Removes check for release notes labels and sev Can merge both ghstack and normal PRs. Be warned though, PRs merged this way will show up as "closed" instead of "merged" on the github UI Future work: move trymerge + dependencies to test-infra (or download from pytorch), make sure its repo agnostic Depends on pytorch/test-infra#5312
For repos that want to merge via
@pytorchbot merge, the repo should putmergebot: Truein theirpytorch-probot.ymlIn preparation for letting torchao trigger their own merges through bot
Risks:
If we can't query for the config for some reason, it will not merge
todo before this gets merged:
Add
mergebot: Truein pytorch's config file