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

Automatically resolve merge conflicts of class definitions? #9

Open
DamienCassou opened this issue Sep 9, 2015 · 3 comments
Open

Comments

@DamienCassou
Copy link
Contributor

In the readme, I can see:

Note that the merge driver will, under some cases, hide conflicts (or resolve them automatically) when a class definition was changed between branches (addition or removal of an instance variable)

This is frightening. Could you please explain a bit more what happens in which case? Can I trust the tool? For example, does the tool search for instance variable usage to decide how to merge addition/removal of instance variables?

@ThierryGoubier
Copy link
Owner

2015-09-09 14:26 GMT+02:00 Damien Cassou [email protected]:

In the readme, I can see:

Note that the merge driver will, under some cases, hide conflicts (or
resolve them automatically) when a class definition was changed between
branches (addition or removal of an instance variable)

This is frightening. Could you please explain a bit more what happens in
which case? Can I trust the tool? For example, does the tool search for
instance variable usage to decide how to merge addition/removal of instance
variables?

Well, it simply takes the union of both definitions, which is the safest
path (i.e. any instance variable defined in one of the branch is kept in
the merge).

So nothing frightening usually, but you may want to do it by hand. You
solve that by not telling the merge driver to take care of class definition
files, which will leave you with a conflict during the merge, conflict that
you will have to solve by other means (git mergetool, by hand, or inside
tODE).

Remember simply that, until the conflict is solved, the package can't be
loaded by filetree or gitfiletree.

And remember again that git is perfectly happy to commit a class definition
with the conflict inside it, which will result in an unreadable version for
your package inside the git history.

Thierry

@DamienCassou
Copy link
Contributor Author

Thierry Goubier [email protected] writes:

Well, it simply takes the union of both definitions, which is the safest
path (i.e. any instance variable defined in one of the branch is kept in
the merge).

does it mean that if a branch removes an inst var and if another branch
adds another inst var, we will get both in the merge? This seems wrong.
The problem is that the merge will succeed and the unused inst var will
go unnoticed.

So nothing frightening usually, but you may want to do it by hand. You
solve that by not telling the merge driver to take care of class definition
files, which will leave you with a conflict during the merge, conflict that
you will have to solve by other means (git mergetool, by hand, or inside
tODE).

this seems right.

Remember simply that, until the conflict is solved, the package can't be
loaded by filetree or gitfiletree.

And remember again that git is perfectly happy to commit a class definition
with the conflict inside it, which will result in an unreadable version for
your package inside the git history.

if you commit such an unresolved conflict and you realize in Pharo that
you can't load it, is it still possible to fix the problem in git
(either with rebase or with a new commit) ? Will Pharo still be happy if
it can't load an old version in the history?

Damien Cassou
http://damiencassou.seasidehosting.st

"Success is the ability to go from one failure to another without
losing enthusiasm." --Winston Churchill

@ThierryGoubier
Copy link
Owner

2015-09-09 15:38 GMT+02:00 Damien Cassou [email protected]:

Thierry Goubier [email protected] writes:

Well, it simply takes the union of both definitions, which is the safest
path (i.e. any instance variable defined in one of the branch is kept in
the merge).

does it mean that if a branch removes an inst var and if another branch
adds another inst var, we will get both in the merge? This seems wrong.
The problem is that the merge will succeed and the unused inst var will
go unnoticed.

Yes. This is the problem.

However, in the state of our tools, leaving to git to leave us a conflct is
even worse. You end up on the command line, without anything to guide you
(branch A still refer to that instance variable even if branch B has
removed it) and a fantastic ability to create an unloadable package :(

So nothing frightening usually, but you may want to do it by hand. You
solve that by not telling the merge driver to take care of class
definition
files, which will leave you with a conflict during the merge, conflict
that
you will have to solve by other means (git mergetool, by hand, or inside
tODE).

this seems right.

IMHO, without the tODE tools, I disagree. Once you have the tODE tools,
then I'd say yes.

Fixing smalltalk conflicts with git tools is error-prone.

Remember simply that, until the conflict is solved, the package can't be
loaded by filetree or gitfiletree.

And remember again that git is perfectly happy to commit a class
definition
with the conflict inside it, which will result in an unreadable version
for
your package inside the git history.

if you commit such an unresolved conflict and you realize in Pharo that
you can't load it, is it still possible to fix the problem in git
(either with rebase or with a new commit) ? Will Pharo still be happy if
it can't load an old version in the history?

Yes. You can fix it in git and it won't matter. Git tools in Pharo have to
/ are written to cope with that, because the condition is present in
existing git repositories.

However, the filetree code is unaware of that and will fail upon loading if
you select the wrong commit.

Thierry

Damien Cassou
http://damiencassou.seasidehosting.st

"Success is the ability to go from one failure to another without
losing enthusiasm." --Winston Churchill


Reply to this email directly or view it on GitHub
#9 (comment)
.

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

2 participants