-
Notifications
You must be signed in to change notification settings - Fork 2
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
[GH Request] Allow edx-transifex-bot to merge to edx-platform without review #940
Comments
Thank you for your report! @openedx/axim-oncall will triage within a business day. Simple requests usually take 2-3 business days to resolve; more complex requests could take longer. |
We'd like this too, but we're trying to find a way of doing it with giving blanket admin rights back to the bot. I've just granted the bot an exception to the PR rule: It's a little counterintuitive, but I think this might allow it to merge PRs without approval. Could you re-run the job and see if it works? If it works, we can apply to this uniformly to all repos. By the way, I'm looking into a similar issue for credentials repos, but it seems like there's also a secondary issue going on there, and I'm still waiting for logs in order to diagnose it. |
Ugh, the job is failing for unrelated reasons. Might take a bit. |
No dice openedx/edx-platform#33526 |
Ah, bummer. I've made the bot an admin on edx-platform, can you try again @rgraber ? |
@rgraber I just realized I didn't hit "Save" when I tried this: #940 (comment) 🤦🏻 I've revoked admin permission from the bot again, but I've actually modified the branch protection rule this time. Try now, and if it still doesn't work, then I'll definitely give the bot admin access again. |
No dice, it's still stuck on review |
Is this just a permissions issue or do we also need to put back additional logic that says "automerge these PRs?" |
OK, the branch protection exemption definitely doesn't work like I'd hoped then. I've made the bot an admin again, can you try one last time?
This will be the first time we're trying with the bot having its full admin rights back on edx-platform. So I think this will do it, but if it doesn't, then maybe there's additional logic we need to figure out. |
Now I'm getting |
I wonder if it's an API key thing, where the key doesn't have permission to get the commit and so it comes back null or something. It's a terrible design choice but it wouldn't surprise me. |
Ack. There's a couple more permutations we can try on this side:
I'm hoping one of those works so we don't have to dive deeper into debugging this deprecated job 😛 I just did (1), can you try again? |
That seems to have worked, although I did manually rerun some of the unit tests when they failed on connection blips. I think we can close this for now, and reopen if it fails again. |
Phew, thanks for your help. |
Reporting something on this closed issue that I believe is an instance of the same problem: LRMFE PR #218 passes all of its checks but fails to merge in Jenkins, I'm not reopening this ticket myself because I'm not sure, but @kdmccormick, i wanted to flag for your attention. |
Thanks @deborahgu , looks like the same exact problem. I'll take a look. |
@kdmccormick is this issue still unresolved? |
@kdmccormick checking back in on this, can this be closed or is there still more work to be done? |
I'm sorry, I don't know, and I can't do any work on it this week. |
@rgraber do you know if this is still an issue? |
Actually, looking at the PR history on edx-platform I see that there was an auto merge of translations last week and it's been fine for a while so I'm gonna close this ticket. |
Firm Name
2U
Urgency
Low (2 weeks)
Requested Change
We would like the edx-transifex-bot to be able to merge its own PRs into edx-platform without review.
Reasoning
There is a Jenkins job in 2U that creates these PRs. It used to merge them automatically but since the recent change to edx-platform permissions, the job has started failing because the PRs aren't merged. It seems like unnecessary toil to have 2U or Axim employees go in and manually merge translations PRs when they happen.
The text was updated successfully, but these errors were encountered: