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

Added --assume-in-merge option for check-merge-conflict #301

Merged
merged 1 commit into from
Jun 26, 2018
Merged

Added --assume-in-merge option for check-merge-conflict #301

merged 1 commit into from
Jun 26, 2018

Conversation

vinayinvicible
Copy link
Contributor

Fixes #300

Copy link
Member

@asottile asottile left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

otherwise looks great 👍

args = parser.parse_args(argv)

if not is_in_merge():
if not is_in_merge(args):
Copy link
Member

@asottile asottile Jun 26, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

probably can leave is_in_merge alone and just do:

if not is_in_merge() and not args.assume_in_merge:
    return 0

(oops, sorry, let's also change the argument to --assume-in-merge now that I'm reading the code /o\)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fixed

@vinayinvicible vinayinvicible changed the title Added --assume-in-conflict option for check-merge-conflict Added --assume-in-merge option for check-merge-conflict Jun 26, 2018
assert detect_merge_conflict(['README.md']) == 0
f = tmpdir.join('README.md')
f.write('problem\n=======\n')
assert detect_merge_conflict([str(f.realpath())]) == 0
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can just use f.strpath here instead of str(f.realpath())

thanks for fixing this test too 👍

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

strpath is not officially documented. this is why I've used realpath

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

interesting! str(f) will work too. I'll work on getting that documented from the py side 😆

@asottile
Copy link
Member

to fix windows, you can .write_binary(b'...') -- newlines are getting mangled so it's not seeing the \n

@asottile
Copy link
Member

Thanks @vinayinvicible this is awesome 🎉

@vinayinvicible
Copy link
Contributor Author

Thanks for instant feedbacks 👍

@vinayinvicible vinayinvicible deleted the force-merge branch June 26, 2018 18:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants