A new rule that makes sure the user have explained the reason of reverting#32
Closed
tehraninasab wants to merge 17 commits intonblockchain:masterfrom
Closed
A new rule that makes sure the user have explained the reason of reverting#32tehraninasab wants to merge 17 commits intonblockchain:masterfrom
tehraninasab wants to merge 17 commits intonblockchain:masterfrom
Conversation
Member
|
@realmarv CI is red |
knocte
reviewed
Nov 15, 2022
commitlintplugins.test.ts
Outdated
|
|
||
|
|
||
| test('proper-revert-message3', () => { | ||
| let commitMsgWithProperRevertMessage = 'Revert "add abbreviations.ts"'; |
Member
There was a problem hiding this comment.
ah damn I'm reading test3 not test2, sorry, let me read test2
Member
|
Ignore all my comments above, let's rebase this PR first. |
knocte
reviewed
Nov 17, 2022
tehraninasab
force-pushed
the
properRevertMessage-new
branch
from
b61c426 to
fde940c
Compare
Contributor
Author
Done |
knocte
reviewed
Nov 17, 2022
Contributor
Author
|
Should it be without a body?
…On Thu, 17 Nov 2022 at 13:21, Andres G. Aragoneses ***@***.***> wrote:
***@***.**** commented on this pull request.
------------------------------
In commitlintplugins.test.ts
<#32 (comment)>
:
> + expect(properRevertMessage1.status).not.toBe(0);
+});
+
+
+test('proper-revert-message2', () => {
+ let commitMsgWithProperRevertMessage =
+ 'Revert "add abbreviations.ts"\n\n' +
+ 'This reverts commit 0272f587c7eece147e8d1756116b0b43e11c34ac\n' +
+ 'because/otherwise bla bla.'
+ let properRevertMessage2 = runCommitLintOnMsg(commitMsgWithProperRevertMessage);
+ expect(properRevertMessage2.status).toBe(0);
+});
+
+
+test('proper-revert-message3', () => {
+ let commitMsgWithProperRevertMessage = 'Revert "add abbreviations.ts"';
@realmarv <https://github.com/realmarv> let's add a new test that has the
commit message "Revert .NET6 migration", and it should have
assert(status).toBe(0)
—
Reply to this email directly, view it on GitHub
<#32 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AL6SKIXMFBU4CGMNC4TKU7LWIX52HANCNFSM6AAAAAASA4QFU4>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
Member
Yes, just a title that says "Revert .NET6 upd as it broke CI" |
Contributor
Author
Done |
knocte
reviewed
Nov 17, 2022
tehraninasab
force-pushed
the
properRevertMessage-new
branch
3 times, most recently
from
1b6d4ca to
fd72e45
Compare
knocte
reviewed
Nov 18, 2022
knocte
reviewed
Nov 18, 2022
commitlint.config.ts
Outdated
| if (body !== null) { | ||
| let bodyStr = convertAnyToString(body, "body").trim(); | ||
| let lines = bodyStr.split('\n'); | ||
| offence = lines[0].match(/^This reverts commit [^ ]{40}.$/) !== null && lines.length == 1; |
Member
There was a problem hiding this comment.
@realmarv are you sure you don't need to escape the '.' here? AFAIU, dots in regexes act as any character, but here I think you're trying to match it with a real dot
Member
|
You didn't push your branch with push1by1. Also, this PR needs a rebase. |
We need to make sure that the commit message is a revert commit message.
The user may explain the reason of reverting in the title.
The reason of reverting might be at the end of the first line.
The reason of reverting might be in the title.
tehraninasab
force-pushed
the
properRevertMessage-new
branch
from
21bf71f to
7003a14
Compare
Contributor
Author
Done |
knocte
reviewed
Jan 9, 2023
| 'title-uppercase': [RuleStatus.Error, 'always'], | ||
| 'proper-revert-message': [RuleStatus.Error, 'always'], | ||
| }, | ||
| // Commitlint automatically ignores some kinds of commits like Revert commit messages. |
Member
There was a problem hiding this comment.
2 last things to do in this PR:
- (Nit) Add an EOL before this comment above, to visually separate it better from the rule configuration list.
- Remove the TODO comment related to this PR, at line 363.
Contributor
Author
|
This PR is closed and replaced with #68 because some commits needed to be squashed and rebased. |
Member
|
This PR should also remove one TODO line from commitling.config.ts. |
Member
|
Oops, this one is closed, sorry. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.