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

.github/workflows/rebase.yml: remove #126192

Merged
merged 1 commit into from Jun 9, 2021
Merged

.github/workflows/rebase.yml: remove #126192

merged 1 commit into from Jun 9, 2021

Conversation

zowoq
Copy link
Contributor

@zowoq zowoq commented Jun 8, 2021

It's kinda broken half the time and I've given up on trying to fix it as I can't get a response in #124560 so it's better if we just remove it entirely.

It only works sometimes and we're unable to fix it.
@Mic92
Copy link
Member

Mic92 commented Jun 8, 2021

It worked better for me than my manual attempts of rebasing...

@Mic92
Copy link
Member

Mic92 commented Jun 8, 2021

Just removing the action because you did not get a response, does not seem like a good course of action.

@zowoq
Copy link
Contributor Author

zowoq commented Jun 8, 2021

Just removing the action because you did not get a response, does not seem like a good course of action.

If I can't even get a response in a reasonable time how long is going to take to actually get the token added and the PR merged?

@zowoq
Copy link
Contributor Author

zowoq commented Jun 9, 2021

So this actually hasn't been working for two weeks, it has been triggered ~25 times and failed every time.

I'm not going to spend any time fixing the current implementation considering I've been trying to push the rewrite forward.

Removing it seems like the best option.

@domenkozar
Copy link
Member

Is there something preventing us to use an existing action like https://github.com/cirrus-actions/rebase

@github-actions
Copy link
Contributor

github-actions bot commented Jun 9, 2021

Failed to rebase

@zowoq
Copy link
Contributor Author

zowoq commented Jun 9, 2021

Is there something preventing us to use an existing action like

Yes, it doesn't rebase across branches.

@domenkozar
Copy link
Member

Is there something preventing us to use an existing action like

Yes, it doesn't rebase across branches.

Could you explain that workflow, I'm not sure what you're referring to.

@zowoq
Copy link
Contributor Author

zowoq commented Jun 9, 2021

Rebasing from master to {staging,staging-next,whatever}. The action you linked will only rebase on the same branch, e.g. master on master.

@zowoq
Copy link
Contributor Author

zowoq commented Jun 9, 2021

Even if we used another action it would still really need another token as the default token doesn't trigger PR actions which is why the current rebase command is using a close/reopen workaround.

@domenkozar
Copy link
Member

Rebasing from master to {staging,staging-next,whatever}. The action you linked will only rebase on the same branch, e.g. master on master.

Isn't that handled by changing the base branch? I wonder if it's really possible to do automatically if two branches diverge.

@zowoq
Copy link
Contributor Author

zowoq commented Jun 9, 2021

# This action allows people with write access to the repo to rebase a PRs base branch
# by commenting `/rebase ${branch}` on the PR while avoiding CODEOWNER notifications.

@github-actions
Copy link
Contributor

github-actions bot commented Jun 9, 2021

Failed to rebase

@zowoq zowoq merged commit 1ea409a into NixOS:master Jun 9, 2021
@zowoq zowoq deleted the rm-rebase branch June 9, 2021 13:44
@domenkozar
Copy link
Member

@zowoq If no such action exists yet, I'd really encourage to write on outside of nixpkgs and then we can just use it here?

@zowoq
Copy link
Contributor Author

zowoq commented Jun 9, 2021

If no such action exists yet, I'd really encourage to write on outside of nixpkgs and then we can just use it here?

I don't see that it would be useful anywhere else except in this repo. I also don't want to maintain an external repo for ~120 loc. If it didn't need to be in .github/workflows to be useable as an action it would be in maintainers/scripts.

@domenkozar Are you going to address my initial comment in (#124560 (comment)) asking about adding a non-default token?

If I can't get an affirmative response about that there really isn't anything else to discuss here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants