jonvex commented on code in PR #13883:
URL: https://github.com/apache/hudi/pull/13883#discussion_r2342289585
##########
scripts/pr_compliance.py:
##########
@@ -28,10 +29,43 @@
# #
# # # # # # # # # # # # # # # # # # # # # # # # # # # # # # # # # # # # # #
+# Allowed PR title prefixes - can be updated to add/remove prefixes as needed
+ALLOWED_PREFIXES = [
+ r"\[HUDI-[0-9]{1,}\]", # HUDI JIRA issue format
+ r"\[MINOR\]", # Minor changes
+ r"\(feat\)", # A new feature
+ r"\(fix\)", # A bug fix
+ r"\(docs\)", # Documentation changes only
+ r"\(style\)", # Code style, formatting, or lint-only changes (no logic
impact)
+ r"\(refactor\)", # Code changes that neither fix a bug nor add a feature
(e.g., cleanups, restructuring)
+ r"\(perf\)", # Performance improvements
+ r"\(test\)", # Adding or correcting tests
+ r"\(chore\)", # Tooling, build system, CI/CD, or maintenance tasks
+ r"\(improvement\)", # Enhancements to existing functionality (aligns with
past JIRA usage)
+ r"\(blocker\)", # Critical issues that block a release (rare, reserved
use)
+ r"\(security\)", # Security-related fixes or improvements
+ r"\(breaking\)" # Use alongside others (e.g., (feat) or (refactor)) to
indicate a breaking change
+]
#validator for titles
def validate_title(title: str):
- return len(re.findall(r'(\[HUDI\-[0-9]{1,}\]|\[MINOR\])',title)) == 1
+ # Create regex pattern from allowed prefixes (without capture groups to
avoid tuples)
+ pattern = r'(?:' + '|'.join(ALLOWED_PREFIXES) + r')'
+ matches = re.findall(pattern, title)
+
+ # Allow exactly one prefix, OR allow combinations with (breaking)
+ if len(matches) == 1:
+ # Don't allow standalone (breaking)
+ return "(breaking)" not in matches
+ elif len(matches) == 2:
+ # Check if one of the matches is (breaking) and the other is (feat) or
(refactor)
+ has_breaking = "(breaking)" in matches
+ has_feat = "(feat)" in matches
+ has_refactor = "(refactor)" in matches
+
+ return has_breaking and (has_feat or has_refactor)
Review Comment:
xor?
##########
scripts/pr_compliance.py:
##########
@@ -28,10 +29,43 @@
# #
# # # # # # # # # # # # # # # # # # # # # # # # # # # # # # # # # # # # # #
+# Allowed PR title prefixes - can be updated to add/remove prefixes as needed
+ALLOWED_PREFIXES = [
+ r"\[HUDI-[0-9]{1,}\]", # HUDI JIRA issue format
+ r"\[MINOR\]", # Minor changes
+ r"\(feat\)", # A new feature
+ r"\(fix\)", # A bug fix
+ r"\(docs\)", # Documentation changes only
+ r"\(style\)", # Code style, formatting, or lint-only changes (no logic
impact)
+ r"\(refactor\)", # Code changes that neither fix a bug nor add a feature
(e.g., cleanups, restructuring)
+ r"\(perf\)", # Performance improvements
+ r"\(test\)", # Adding or correcting tests
+ r"\(chore\)", # Tooling, build system, CI/CD, or maintenance tasks
+ r"\(improvement\)", # Enhancements to existing functionality (aligns with
past JIRA usage)
+ r"\(blocker\)", # Critical issues that block a release (rare, reserved
use)
+ r"\(security\)", # Security-related fixes or improvements
+ r"\(breaking\)" # Use alongside others (e.g., (feat) or (refactor)) to
indicate a breaking change
+]
#validator for titles
def validate_title(title: str):
- return len(re.findall(r'(\[HUDI\-[0-9]{1,}\]|\[MINOR\])',title)) == 1
+ # Create regex pattern from allowed prefixes (without capture groups to
avoid tuples)
+ pattern = r'(?:' + '|'.join(ALLOWED_PREFIXES) + r')'
+ matches = re.findall(pattern, title)
+
+ # Allow exactly one prefix, OR allow combinations with (breaking)
+ if len(matches) == 1:
+ # Don't allow standalone (breaking)
+ return "(breaking)" not in matches
+ elif len(matches) == 2:
+ # Check if one of the matches is (breaking) and the other is (feat) or
(refactor)
+ has_breaking = "(breaking)" in matches
+ has_feat = "(feat)" in matches
+ has_refactor = "(refactor)" in matches
+
+ return has_breaking and (has_feat or has_refactor)
Review Comment:
oh I guess since len(matches) == 2 then it can only be 1
##########
scripts/pr_compliance.py:
##########
@@ -385,33 +459,57 @@ def validate(self):
return False
self.section.error("","", "Please make sure you have filled out the
template correctly. You can find a blank template in
/.github/PULL_REQUEST_TEMPLATE.md")
return False
+
+ # Check if body contains GitHub issue link
+ def _has_github_issue_link(self):
Review Comment:
Maybe we should unit test this?
##########
scripts/pr_compliance.py:
##########
@@ -28,10 +29,43 @@
# #
# # # # # # # # # # # # # # # # # # # # # # # # # # # # # # # # # # # # # #
+# Allowed PR title prefixes - can be updated to add/remove prefixes as needed
+ALLOWED_PREFIXES = [
+ r"\[HUDI-[0-9]{1,}\]", # HUDI JIRA issue format
+ r"\[MINOR\]", # Minor changes
+ r"\(feat\)", # A new feature
+ r"\(fix\)", # A bug fix
+ r"\(docs\)", # Documentation changes only
+ r"\(style\)", # Code style, formatting, or lint-only changes (no logic
impact)
+ r"\(refactor\)", # Code changes that neither fix a bug nor add a feature
(e.g., cleanups, restructuring)
+ r"\(perf\)", # Performance improvements
+ r"\(test\)", # Adding or correcting tests
+ r"\(chore\)", # Tooling, build system, CI/CD, or maintenance tasks
+ r"\(improvement\)", # Enhancements to existing functionality (aligns with
past JIRA usage)
+ r"\(blocker\)", # Critical issues that block a release (rare, reserved
use)
+ r"\(security\)", # Security-related fixes or improvements
+ r"\(breaking\)" # Use alongside others (e.g., (feat) or (refactor)) to
indicate a breaking change
+]
#validator for titles
def validate_title(title: str):
- return len(re.findall(r'(\[HUDI\-[0-9]{1,}\]|\[MINOR\])',title)) == 1
+ # Create regex pattern from allowed prefixes (without capture groups to
avoid tuples)
+ pattern = r'(?:' + '|'.join(ALLOWED_PREFIXES) + r')'
+ matches = re.findall(pattern, title)
+
+ # Allow exactly one prefix, OR allow combinations with (breaking)
+ if len(matches) == 1:
+ # Don't allow standalone (breaking)
+ return "(breaking)" not in matches
+ elif len(matches) == 2:
+ # Check if one of the matches is (breaking) and the other is (feat) or
(refactor)
+ has_breaking = "(breaking)" in matches
+ has_feat = "(feat)" in matches
+ has_refactor = "(refactor)" in matches
+
+ return has_breaking and (has_feat or has_refactor)
Review Comment:
has_feat xor has_refactor? can it be both?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
