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

Manual backports with compatibility patches #468

Closed
BridgeAR opened this issue Aug 6, 2019 · 5 comments
Closed

Manual backports with compatibility patches #468

BridgeAR opened this issue Aug 6, 2019 · 5 comments
Assignees

Comments

@BridgeAR
Copy link
Member

BridgeAR commented Aug 6, 2019

Regular manual backports are currently done by changing the actual code change and applying the necessary changes directly to the backported commit.

I would like to change that to always use the original commit without any changes and to add another commit afterwards that works as compatibility patch. That way it's

a) easier to review the changes and accept the PR faster
b) potential conflicts will normally happen with the compatibility patches instead of the original commit.

Especially the second point could reduce the amount of work necessary in case of conflicts (it would be easier to determine what's wrong).

I can think of one downside though: in theory bisecting could become more difficult because the original commit could cause problems. It should likely mostly not be an issue besides broken builds.

@MylesBorins
Copy link
Contributor

MylesBorins commented Aug 6, 2019 via email

@BridgeAR
Copy link
Member Author

BridgeAR commented Aug 6, 2019

Example:

Backport includes the changes in the same commit: nodejs/node#29013
Backport with separate compatibility patch: nodejs/node#29011

@targos
Copy link
Member

targos commented Aug 6, 2019

I'm -1 on landing commits with conflict markers.
What we can maybe ask people to do is open the PR in two commits and squash them while landing.

@BridgeAR
Copy link
Member Author

BridgeAR commented Aug 6, 2019

@targos

What we can maybe ask people to do is open the PR in two commits and squash them while landing.

I think that is a good middle ground. That way it's already easier to review.

@MylesBorins

How would this work when there is a conflict?

It would depend on how the conflict came up and it's somewhat difficult to predict that. I think recommending to open a PR with the changes as separate commit and to squash those while landing is already improvement enough that we can focus on that instead of my original proposal.

@BethGriggs
Copy link
Member

Closing, as this is fairly old now and I don't believe we're pursuing this idea further. Please reopen if you feel closing is/was inappropriate here.

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

No branches or pull requests

4 participants