From c1e675439ac5769e841b4d892ee2411a27186982 Mon Sep 17 00:00:00 2001 From: Ravi Mistry Date: Tue, 6 Sep 2016 13:11:54 -0400 Subject: [PATCH] Follow git standard when creating reverts Context is in https://bugs.chromium.org/p/gerrit/issues/detail?id=4492 Change-Id: If6b93661de2a3225418b525dc8afdb1711f62928 --- .../gr-confirm-revert-dialog.js | 14 ++++----- .../gr-confirm-revert-dialog_test.html | 30 +++++++++++++++---- 2 files changed, 30 insertions(+), 14 deletions(-) diff --git a/polygerrit-ui/app/elements/change/gr-confirm-revert-dialog/gr-confirm-revert-dialog.js b/polygerrit-ui/app/elements/change/gr-confirm-revert-dialog/gr-confirm-revert-dialog.js index f38ef743b2..2eb66463ca 100644 --- a/polygerrit-ui/app/elements/change/gr-confirm-revert-dialog/gr-confirm-revert-dialog.js +++ b/polygerrit-ui/app/elements/change/gr-confirm-revert-dialog/gr-confirm-revert-dialog.js @@ -36,18 +36,16 @@ populateRevertMessage: function(message) { // Figure out what the revert title should be. var originalTitle = message.split('\n')[0]; - var revertTitle = 'Revert of ' + originalTitle; - if (originalTitle.startsWith('Revert of ')) { - revertTitle = 'Reland of ' + - originalTitle.substring('Revert of '.length); - } else if (originalTitle.startsWith('Reland of ')) { - revertTitle = 'Revert of ' + - originalTitle.substring('Reland of '.length); - } + var revertTitle = 'Revert "' + originalTitle + '"'; + // Figure out what the revert commit message should be. + var commitRegex = /\n{1,2}\nChange-Id: (\w+)\n/gm; + var match = commitRegex.exec(message); + var revertCommitText = 'This reverts commit ' + match[1] + '.'; // Add '> ' in front of the original commit text. var originalCommitText = message.replace(/^/gm, '> '); this.message = revertTitle + '\n\n' + + revertCommitText + '\n\n' + 'Reason for revert: \n\n' + 'Original issue\'s description:\n' + originalCommitText; }, diff --git a/polygerrit-ui/app/elements/change/gr-confirm-revert-dialog/gr-confirm-revert-dialog_test.html b/polygerrit-ui/app/elements/change/gr-confirm-revert-dialog/gr-confirm-revert-dialog_test.html index 3b3851ad28..521aeefaf0 100644 --- a/polygerrit-ui/app/elements/change/gr-confirm-revert-dialog/gr-confirm-revert-dialog_test.html +++ b/polygerrit-ui/app/elements/change/gr-confirm-revert-dialog/gr-confirm-revert-dialog_test.html @@ -40,21 +40,39 @@ limitations under the License. test('single line', function() { assert.isNotOk(element.message); - element.populateRevertMessage('one line commit'); - var expected = 'Revert of one line commit\n\n' + + element.populateRevertMessage('one line commit\n\nChange-Id: abcdefg\n'); + var expected = 'Revert "one line commit"\n\n' + + 'This reverts commit abcdefg.\n\n' + 'Reason for revert: \n\n' + 'Original issue\'s description:\n' + - '> one line commit'; + '> one line commit\n> \n' + + '> Change-Id: abcdefg\n> '; assert.equal(element.message, expected); }); test('multi line', function() { assert.isNotOk(element.message); - element.populateRevertMessage('many lines\ncommit\n\nmessage\n'); - var expected = 'Revert of many lines\n\n' + + element.populateRevertMessage( + 'many lines\ncommit\n\nmessage\n\nChange-Id: abcdefg\n'); + var expected = 'Revert "many lines"\n\n' + + 'This reverts commit abcdefg.\n\n' + 'Reason for revert: \n\n' + 'Original issue\'s description:\n' + - '> many lines\n> commit\n> \n> message\n> '; + '> many lines\n> commit\n> \n> message\n> \n' + + '> Change-Id: abcdefg\n> '; + assert.equal(element.message, expected); + }); + + test('revert a revert', function () { + assert.isNotOk(element.message); + element.populateRevertMessage( + 'Revert "one line commit"\n\nChange-Id: abcdefg\n'); + var expected = 'Revert "Revert "one line commit""\n\n' + + 'This reverts commit abcdefg.\n\n' + + 'Reason for revert: \n\n' + + 'Original issue\'s description:\n' + + '> Revert "one line commit"\n> \n' + + '> Change-Id: abcdefg\n> '; assert.equal(element.message, expected); }); });