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

Configuration of conflict resolver #55

Closed
jacobilsoe opened this issue Sep 7, 2015 · 18 comments
Closed

Configuration of conflict resolver #55

jacobilsoe opened this issue Sep 7, 2015 · 18 comments

Comments

@jacobilsoe
Copy link
Contributor

It would be nice to have an option to control what change sets the conflict resolver picks to accept.

Right now, only change sets belonging to the same author or change sets with "merge" in the comment text are accepted together. I have several examples where change sets from different authors need to be accepted together to resolve a conflict.

IMO, the resolver should continue accepting change sets together until there are no more change sets. - and only then give up. It should succeed at some point.

@WtfJoke
Copy link
Member

WtfJoke commented Sep 8, 2015

Me and @ohumbel discussed about this issue today.
His (I had another idea, but his was better IMO) idea was to define a variable number of changesets, which are accepted together to resolve conflicts.

A suitable default value could be 10.
In your case, you could set this value to 1000 or something like that.

What I fear a bit is that under some circumstances unnecessary commands get spammed. For example, assume that the connection to the server get lost. In this case it will go through all (1000) and in the end it still fails, and you dont know why.

If there are only 10 changeset you might figure out, what is responsible for the conflict.

Also I think there are conflicts, which not get resolved by accepting multiple changeset (at least I think I got some of them in our companies-repo).

Is this approach suitable for you jacob?

@jacobilsoe
Copy link
Contributor Author

Yes, that would be a fine solution. 😄 I haven't seen the need to accept more than 10-15 change sets together. The important thing is that change sets from different users can be accepted together.

@jacobilsoe
Copy link
Contributor Author

Are you going to work on it, or should I have a go?

@ohumbel
Copy link
Member

ohumbel commented Sep 14, 2015

I plan to, but I want to solve issue #52 first, so that only change sets from the same component are considered for conflict resolution. So please feel free to work on this, I'll take care of merging ;-)

@WtfJoke
Copy link
Member

WtfJoke commented Sep 14, 2015

assignes to @jacobilsoe

@jacobilsoe
Copy link
Contributor Author

So with this new option I suppose we no longer need the arereleatedmergechangesets method. Only the number of change sets specified in the option is needed. The number of change sets accepted together will continually increase until it success or the max value is reached.

@WtfJoke
Copy link
Member

WtfJoke commented Sep 14, 2015

Yes, exactly

@ohumbel
Copy link
Member

ohumbel commented Sep 14, 2015

...and with #52, I'll feed you with the change sets from the right component.
Together we'll be strong 👍

@jacobilsoe
Copy link
Contributor Author

Is it just me or do you also have 7 failing tests? I run tests like this: python.exe -m unittest discover -s tests and e.g. the test test_splitoutputofgitstatusz_filterprefix_double_question (test_gitFunctions.GitFunctionsTestCase) fails with: FileNotFoundError: [Errno 2] No such file or directory: './resources/test_ignore_git_status_z.txt' It looks like some relative path is wrong.

@ohumbel
Copy link
Member

ohumbel commented Sep 15, 2015

If I run them like you, I also get those 7 errors.
I usually run the whole test suite from within PyCharm, which seems to handle those relative file paths.
I'll try to rewrite them for both use cases.

@ohumbel
Copy link
Member

ohumbel commented Sep 15, 2015

The tests now pass fine from python.exe -m unittest discover -s tests

@jacobilsoe
Copy link
Contributor Author

Thanks!

@jacobilsoe
Copy link
Contributor Author

Just tested on a problematic stream, and for the first time the migration succeeded without requiring any manual intervention. This is nice. 😄

@ohumbel
Copy link
Member

ohumbel commented Sep 15, 2015

Very cool ⭐ ✨

@WtfJoke
Copy link
Member

WtfJoke commented Sep 15, 2015

Nice, glad that it works out as expected. 😄

@WtfJoke
Copy link
Member

WtfJoke commented Sep 17, 2015

The tests now pass fine from python.exe -m unittest discover -s tests

Now it works also in windows, 💯 it wasnt implemented os independent (there were hardcoded \). see 71edb05

@ohumbel
Copy link
Member

ohumbel commented Sep 17, 2015

Thanks - my fault only testing on Linux...

@WtfJoke
Copy link
Member

WtfJoke commented Sep 18, 2015

No big deal. I usually dont test on linux as well 😛

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

No branches or pull requests

3 participants