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

Fix 3954 quotes in merge commit message #8042

Merged
merged 2 commits into from Apr 28, 2020

Conversation

RussKie
Copy link
Member

@RussKie RussKie commented Apr 26, 2020

Fixes #3954

This work is inspired by #7796 that was abandoned by its author.

Proposed changes

  • Centralise commiit message handling by moving the code from FormCommit that formatted and wrote a commit message into a temp file before a git command was run to CommitMessageManager.
    Leverage the same implementation for FormMergeBranch to write out merge-commit messages.
  • Tighten git-merge log condition

Screenshots

Before

Merge commit message with quotes was failing the merge test "message text" test
image

After

The merge succeeds.
image
image

Test methodology

  • manual tests
  • unit tests

Fixes #3954

Centralise commiit message handling by moving the code from `FormCommit`
that formatted and wrote a commit message into a temp file before a git
command was run to `CommitMessageManager`.

Leverage the same implementation for `FormMergeBranch` to write out
merge-commit messages.

Add tests.
@ghost ghost assigned RussKie Apr 26, 2020
@codecov
Copy link

codecov bot commented Apr 26, 2020

Codecov Report

Merging #8042 into master will increase coverage by 0.00%.
The diff coverage is 87.61%.

@@           Coverage Diff           @@
##           master    #8042   +/-   ##
=======================================
  Coverage   49.60%   49.60%           
=======================================
  Files         844      844           
  Lines       60849    60861   +12     
  Branches    10877    10891   +14     
=======================================
+ Hits        30185    30192    +7     
- Misses      28448    28452    +4     
- Partials     2216     2217    +1     
Flag Coverage Δ
#production 37.42% <65.78%> (-0.02%) ⬇️
#tests 94.87% <100.00%> (+0.04%) ⬆️

@RussKie
Copy link
Member Author

RussKie commented Apr 27, 2020

@msftbot merge in 25 hours

@ghost ghost added the status: auto merge label Apr 27, 2020
@ghost
Copy link

ghost commented Apr 27, 2020

Hello @RussKie!

Because you've given me some instructions on how to help merge this pull request, I'll be modifying my merge approach. Here's how I understand your requirements for merging this pull request:

  • I won't merge this pull request until after the UTC date Tue, 28 Apr 2020 12:24:40 GMT, which is in 1 day and 1 hour

If this doesn't seem right to you, you can tell me to cancel these instructions and use the auto-merge policy that has been configured for this repository. Try telling me "forget everything I just told you".

Copy link
Member

@mstv mstv left a comment

Choose a reason for hiding this comment

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

👍

@RussKie RussKie merged commit 03565bf into master Apr 28, 2020
@RussKie RussKie deleted the fix_3954_quotes_in_merge-commit_message branch April 28, 2020 01:00
@ghost ghost added this to the 4.0 milestone Apr 28, 2020
@RussKie RussKie modified the milestones: 4.0, 3.4 Apr 28, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Commit message field in merge dialog does not escape commit message properly
2 participants