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

Infrastructure: add workflow to pull latest development #6020

Closed
wants to merge 1 commit into from

Conversation

vadi2
Copy link
Member

@vadi2 vadi2 commented Mar 22, 2022

Brief overview of PR changes/additions

Typing a /rebase comment in a PR will pull latest development changes in.

Motivation for adding to Mudlet

It's necessary to do this sometimes, and being able to do it from the web without messing with git is nice.

Other info (issues closed, discussion etc)

Built with https://github.com/marketplace/actions/automatic-rebase

@vadi2 vadi2 requested a review from a team March 22, 2022 12:31
@add-deployment-links
Copy link

add-deployment-links bot commented Mar 22, 2022

Hey there! Thanks for helping Mudlet improve. 🌟

Test versions

You can directly test the changes here:

No need to install anything - just unzip and run.
Let us know if it works well, and if it doesn't, please give details.

@mudlet-machine-account mudlet-machine-account added this to the 4.16.0 milestone Mar 22, 2022
@github-actions
Copy link
Contributor

Messages
✔️

PR type: Infrastructure

Generated by 🚫 dangerJS against 24572c9

@SlySven
Copy link
Member

SlySven commented Mar 22, 2022

🤔 So this rebases a pending PR on top of the current development branch tip does it - or does it merge that branch into the PR? If it does do the former then anyone tracking the PR in their own GH repository (except the PR proposer whose one will be targetted by the rebase) AND their local working repositories must fetch the changes before they pile any further commits onto the PR or risk their working codebase diverging from the "master"...

@SlySven
Copy link
Member

SlySven commented Mar 22, 2022

All in all, I'd recommend that this command only be used (except in exceptional circumstances) by the PR proposer so they do not get the rug pulled out from under them when they find their GH repository no longer reflects their local working copy and they cannot push out further, previously unpublished, commits to there without having to jump through some extra hoops to stash those, pull in the changes, and then rebase their stashed commits onto the new HEAD.

@vadi2
Copy link
Member Author

vadi2 commented Mar 25, 2022

It's called rebase so I believe it rebases. I am OK with others using it as well, on some occasions the PR proposer is no longer active and updating their repository will be fine. Cases where someone gets the rug pulled under them will be rare and unlikely a huge issue when it does happen.

Limiting it to the PR proposer only would defeat the purpose of this improvement - the idea is that someone other than the proposer can update the pull request without having to mess with git.

@SlySven
Copy link
Member

SlySven commented Mar 25, 2022

@vadi2 if you need to update a PR so that it incorporates the changes done in the main development branch then you need to MERGE the development branch into the PR - if you take all the existing commits from the PR and then REBASE them on top of the current development branch you will rewrite the PUBLIC history of the PR and f**k-up the local working repository of anyone else who is tracking/working on the PR separately.

Remember you shouldn't rebase publicly released commits which is what PRs are, as https://git-scm.com/book/en/v2/Git-Branching-Rebasing puts it:

The Perils of Rebasing

Ahh, but the bliss of rebasing isn’t without its drawbacks, which can be summed up in a single line:

Do not rebase commits that exist outside your repository and that people may have based work on.

If you follow that guideline, you’ll be fine. If you don’t, people will hate you, and you’ll be scorned by friends and family.

When you rebase stuff, you’re abandoning existing commits and creating new ones that are similar but different. If you push commits somewhere and others pull them down and base work on them, and then you rewrite those commits with git rebase and push them up again, your collaborators will have to re-merge their work and things will get messy when you try to pull their work back into yours.

tl;dr; Only rebase in private where no-one else can see you!

@vadi2
Copy link
Member Author

vadi2 commented Apr 2, 2022

Let's give this a try and see if it is a problem in practice, before we write it off on theory alone. If anything does go bad, we'll turn this off and will be able to recover thanks to git history.

@SlySven
Copy link
Member

SlySven commented Apr 3, 2022

👎 I repeat - we do not want to rebase PRs onto the development branch as it breaks things - we should be merging (like we already do manually when CI breaks and a fix gets into development) so we do not mess up everyone's history/collaboration on PRs. Note that this has been raised as an issue for the GHA you are proposing: cirrus-actions/rebase#56 and someone there has forked a version that does do merges as well/instead...

@keneanung
Copy link
Member

How about using the built-in feature https://docs.github.com/en/repositories/configuring-branches-and-merges-in-your-repository/configuring-pull-request-merges/managing-suggestions-to-update-pull-request-branches instead of a gh action. That one leaves the choice open to the person using that button, if I get the docs correctly.

@vadi2
Copy link
Member Author

vadi2 commented Apr 4, 2022

Hey, even better! I've enabled that option. Big thanks @keneanung.

@vadi2 vadi2 closed this Apr 4, 2022
@vadi2 vadi2 deleted the add-rebase-workflow branch April 4, 2022 09:37
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 this pull request may close these issues.

4 participants