Skip to content

Comments

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

Merged
asottile merged 1 commit intopre-commit:masterfrom
vinayinvicible:force-merge
Jun 26, 2018
Merged

Added --assume-in-merge option for check-merge-conflict#301
asottile merged 1 commit intopre-commit:masterfrom
vinayinvicible:force-merge

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