Skip to content

Conversation

@nokute78
Copy link
Collaborator

This patch allows = and / for commit message.
e.g.

  • =
    • build: fix build failure with -DFLB_SHARED_LIB=OFF (#3042) patch
  • /
    • build: fix cpack rules and validate flex/bison patch

Enter [N/A] in the box, if an item is not applicable to your change.

Testing
Before we can approve your change; please submit the following in a comment:

  • [N/A] Example configuration file for the change
  • Debug log output from testing the change
  • [N/A] Attached Valgrind output that shows no leaks or memory corruption was found

Documentation

  • [N/A] Documentation required for this feature

Debug output

equal case.

irb(main):001:0> str = 'build: fix build failure with -DFLB_SHARED_LIB=OFF (#3042)'
irb(main):002:0> str.match /^[a-z0-9\-_]+\:[ ]{0,1}[a-z]+[a-zA-Z0-9 \-\.\:_\#\(\)=\/]+$/
=> #<MatchData "build: fix build failure with -DFLB_SHARED_LIB=OFF (#3042)">

slash case.

irb(main):001:0> str = 'build: fix cpack rules and validate flex/bison'
irb(main):002:0> str.match /^[a-z0-9\-_]+\:[ ]{0,1}[a-z]+[a-zA-Z0-9 \-\.\:_\#\(\)=\/]+$/
=> #<MatchData "build: fix cpack rules and validate flex/bison">

Fluent Bit is licensed under Apache 2.0, by submitting this pull request I understand that this code will be released under the terms of that license.

Signed-off-by: Takahiro Yamashita <[email protected]>
@fujimotos
Copy link
Member

Hmm. The commit checker is evidently being too picky. This is the fourth PR
(after #3285, #3281 and #3270) that tries to amend the issue, but the bot
apparently needs a few more tweaks. Using this Python script:

import re
import sys 

pat = re.compile(r'^[a-z0-9\-_]+\:[ ]{0,1}[a-z]+[a-zA-Z0-9 \-\.\:_\#\(\)=\/]+$')
bad = 0 

for n, line in enumerate(sys.stdin):
    line = line.strip()
    if not pat.match(line):
        print("bad commit:", line)
        bad += 1

print("%i/%i rejected (%i%%)" % (bad, n, bad / n * 100))

I can confirm that the checking bot rejects ~1/4 of our historical commits:

$ git log --oneline  --no-merges --decorate=no | cut -d " " -f 2- | python3 a.py
...
bad commit: Core: initialization and 32 bit fixes.
1418/5933 rejected (23%)

Overall, the robot seems to be rejecting tons of valid commits, and actively
making contributors feel miserable.

@nokute78
Copy link
Collaborator Author

@fujimotos nice script!

I modified script and this diff reports 92/6029 rejected (1%)

--- /home/taka/a.py	2021-03-31 11:28:42.256176691 +0900
+++ /home/taka/b.py	2021-03-31 11:28:24.972003619 +0900
@@ -1,7 +1,7 @@
 import re
 import sys 
 
-pat = re.compile(r'^[a-z0-9\-_]+\:[ ]{0,1}[a-z]+[a-zA-Z0-9 \-\.\:_\#\(\)=\/]+$')
+pat = re.compile(r'^[a-z0-9A-Z\-_\s\,\.\/]+\:[ ]{0,1}[a-zA-Z]+[a-zA-Z0-9 \-\.\:_\#\(\)=\/\'\"\,><\+\[\]\!\*\\]+$')
 bad = 0 
 
 for n, line in enumerate(sys.stdin):

@fujimotos
Copy link
Member

I modified script and this diff reports 92/6029 rejected (1%)

@nokute78 Great. Can you submit that pattern as a PR with the
script report attached?

Looking over the new pattern, I'm basically fine with your proposal.

  - allow upper case, space, '.', '.', '/' as prefix.
  - allow upper case character as message first character
  - allow '"', ''', '<', '>', '+', '[', ']', '!' '*' '\' character

Signed-off-by: Takahiro Yamashita <[email protected]>
@nokute78
Copy link
Collaborator Author

nokute78 commented Mar 31, 2021

I added commit to respect #3306 (comment).

By the way, It is against this document
We may need to fix CONTRIBUTING.md.

e.g. Your principal commit message (one line subject) must be prefixed with the core section name in lowercase plus a colon
Refer to this, SSL/TLS: add mbedtls encryption library is not allowed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants