Skip to content
This repository has been archived by the owner on Apr 18, 2022. It is now read-only.

Rebase or merge on conflicting changes with upstream? #109

Closed
Emilgardis opened this issue Sep 20, 2016 · 3 comments
Closed

Rebase or merge on conflicting changes with upstream? #109

Emilgardis opened this issue Sep 20, 2016 · 3 comments
Labels
diff: easy Achievable by a single junior developer with a bit of guidance. pri: normal An issue that is causing a noticeable impact, but can be worked around
Projects
Engine

Comments

@Emilgardis
Copy link
Contributor

Emilgardis commented Sep 20, 2016 •
edited

There has been a few "sync with latest develop" commits lately. (b17b86e (in open pr #99), 266819c, 266819c, 3da54cd, cac16ba, and probably more)
The main problem here is if one should merge or rebase a pull request. Should we come to a agreement on how to handle "rebasable" upstream changes to a pr, or should we let the coder choose for themselves, i.e merge (aka sync with branch) or rebase (rewind, play changes, fastforward, fix possible conflicts)?
@ebkalderon
Copy link
Member

ebkalderon commented Sep 22, 2016 •
edited

@Emilgardis Yes, this is a good point. I think that many of these conflicts arise due to the nested nature of the crates. I don't know of a one size fits all approach to this problem off the top of my head, but I'll definitely think about it. If anyone else has ideas, please share them.

Update: When pulling remote changes to our local machines, we rebase. When merging local changes into the mainline, we merge. Does this sound good?

@ebkalderon ebkalderon added note: help wanted diff: easy Achievable by a single junior developer with a bit of guidance. pri: normal An issue that is causing a noticeable impact, but can be worked around labels Sep 22, 2016
@Emilgardis
Copy link
Contributor Author

Yes, that sounds reasonable. Right now we have sync commits on #99 and #81

I'll make a PR to reflect this in CONTRIBUTING.md

Emilgardis added a commit to Emilgardis/amethyst that referenced this issue Nov 27, 2016
@Emilgardis
Add suggestion per amethyst#109
24a7e4a
Emilgardis added a commit to Emilgardis/amethyst that referenced this issue Nov 27, 2016
@Emilgardis
Add suggestion per amethyst#109
a08c28c
Emilgardis added a commit to Emilgardis/amethyst that referenced this issue Nov 27, 2016
@Emilgardis
Add suggestion per amethyst#109
21ee399
Aceeri added a commit that referenced this issue Nov 30, 2016
@Aceeri
Merge pull request #130 from Emilgardis/mention_pr_pulls
b20c6e7 
Add suggestion per #109
@Emilgardis
Copy link
Contributor Author

21ee399 will in some ways prevent this. Closing issue

anderejd pushed a commit to anderejd/amethyst that referenced this issue Dec 30, 2016
@Emilgardis @anderejd
Add suggestion per amethyst#109
00b2ac2
@ebkalderon ebkalderon added this to Done in Engine Feb 3, 2017
@ebkalderon ebkalderon moved this from Done to Shipped in Engine Feb 7, 2017
@ebkalderon ebkalderon moved this from Shipped to Done in Engine Feb 7, 2017
@ebkalderon ebkalderon moved this from Done to Shipped in Engine Feb 9, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
diff: easy Achievable by a single junior developer with a bit of guidance. pri: normal An issue that is causing a noticeable impact, but can be worked around
Projects
No open projects
Engine
  
Shipped
Milestone
No milestone
Development

No branches or pull requests
2 participants
@ebkalderon @Emilgardis