static_checks: Skip commit message
Skip the commit message when looking for 'No-Docs-Update-Reason:'. In
fact, forbid that line from appearing in commit messages.
Change-Id: I12249db9b738ee5224625bd0b0b3f4ff2a2a82f2
Reviewed-on: https://pigweed-review.googlesource.com/c/infra/recipes/+/68222
Reviewed-by: Wyatt Hepler <hepler@google.com>
Reviewed-by: Ted Pudlik <tpudlik@google.com>
Commit-Queue: Rob Mohr <mohrr@google.com>
diff --git a/recipe_modules/util/api.py b/recipe_modules/util/api.py
index a88cd56..aed0c78 100644
--- a/recipe_modules/util/api.py
+++ b/recipe_modules/util/api.py
@@ -25,6 +25,7 @@
class ChangeWithComments(object):
change = attr.ib()
details = attr.ib()
+ commit_message = attr.ib()
comments = attr.ib()
@@ -56,7 +57,9 @@
).json.output
current_revision = details['revisions'][details['current_revision']]
- comments = [current_revision['commit']['message']]
+ commit_message = current_revision['commit']['message']
+
+ comments = []
for revision in details['revisions'].values():
if revision.get('description'):
@@ -73,7 +76,7 @@
for _, comment_data in comments_result.items():
comments.extend(x['message'] for x in comment_data)
- return ChangeWithComments(change, details, comments)
+ return ChangeWithComments(change, details, commit_message, comments)
def find_matching_comment(self, rx, comments):
"""Find a comment in comments that matches regex object rx."""
diff --git a/recipes/static_checks.expected/docs.commit-message.json b/recipes/static_checks.expected/docs.commit-message-ignored.json
similarity index 91%
rename from recipes/static_checks.expected/docs.commit-message.json
rename to recipes/static_checks.expected/docs.commit-message-ignored.json
index 4ff4903..2c840d0 100644
--- a/recipes/static_checks.expected/docs.commit-message.json
+++ b/recipes/static_checks.expected/docs.commit-message-ignored.json
@@ -246,46 +246,16 @@
},
{
"cmd": [],
- "name": "check docs"
- },
- {
- "cmd": [],
- "name": "check docs.checking comments",
+ "name": "check docs",
"~followup_annotations": [
- "@@@STEP_NEST_LEVEL@1@@@"
+ "@@@STEP_FAILURE@@@"
]
},
{
- "cmd": [],
- "name": "check docs.checking comments.comment (0)",
- "~followup_annotations": [
- "@@@STEP_NEST_LEVEL@2@@@",
- "@@@STEP_SUMMARY_TEXT@MATCH: No-Docs-Update-Reason: foo@@@"
- ]
- },
- {
- "cmd": [],
- "name": "check docs.checking comments.found",
- "~followup_annotations": [
- "@@@STEP_NEST_LEVEL@2@@@"
- ]
- },
- {
- "cmd": [],
- "name": "check docs.found No-Docs-Update-Reason",
- "~followup_annotations": [
- "@@@STEP_NEST_LEVEL@1@@@"
- ]
- },
- {
- "cmd": [],
- "name": "check requires"
- },
- {
- "cmd": [],
- "name": "check dns"
- },
- {
+ "failure": {
+ "failure": {},
+ "humanReason": "Found \"No-Docs-Update-Reason:\" in the commit message which is no longer allowed. Please use a Gerrit comment instead (i.e., click \"Reply\" in the Gerrit UI)."
+ },
"name": "$result"
}
]
\ No newline at end of file
diff --git a/recipes/static_checks.expected/docs.gerrit-comment.json b/recipes/static_checks.expected/docs.gerrit-comment.json
index bf6335a..e130082 100644
--- a/recipes/static_checks.expected/docs.gerrit-comment.json
+++ b/recipes/static_checks.expected/docs.gerrit-comment.json
@@ -259,13 +259,6 @@
"cmd": [],
"name": "check docs.checking comments.comment (0)",
"~followup_annotations": [
- "@@@STEP_NEST_LEVEL@2@@@"
- ]
- },
- {
- "cmd": [],
- "name": "check docs.checking comments.comment (1)",
- "~followup_annotations": [
"@@@STEP_NEST_LEVEL@2@@@",
"@@@STEP_SUMMARY_TEXT@MATCH: No-Docs-Update-Reason: laziness@@@"
]
diff --git a/recipes/static_checks.expected/docs.has_doc_changes.json b/recipes/static_checks.expected/docs.has_doc_changes.json
index b0cd756..65e7ba6 100644
--- a/recipes/static_checks.expected/docs.has_doc_changes.json
+++ b/recipes/static_checks.expected/docs.has_doc_changes.json
@@ -266,13 +266,6 @@
},
{
"cmd": [],
- "name": "check docs.checking comments.comment (1)",
- "~followup_annotations": [
- "@@@STEP_NEST_LEVEL@2@@@"
- ]
- },
- {
- "cmd": [],
"name": "check docs.checking changed files",
"~followup_annotations": [
"@@@STEP_NEST_LEVEL@1@@@",
diff --git a/recipes/static_checks.expected/docs.has_doc_changes_properties.json b/recipes/static_checks.expected/docs.has_doc_changes_properties.json
index a1e1163..1647100 100644
--- a/recipes/static_checks.expected/docs.has_doc_changes_properties.json
+++ b/recipes/static_checks.expected/docs.has_doc_changes_properties.json
@@ -266,13 +266,6 @@
},
{
"cmd": [],
- "name": "check docs.checking comments.comment (1)",
- "~followup_annotations": [
- "@@@STEP_NEST_LEVEL@2@@@"
- ]
- },
- {
- "cmd": [],
"name": "check docs.checking changed files",
"~followup_annotations": [
"@@@STEP_NEST_LEVEL@1@@@",
diff --git a/recipes/static_checks.expected/docs.no_doc_changes.json b/recipes/static_checks.expected/docs.no_doc_changes.json
index ade10e2..32548ed 100644
--- a/recipes/static_checks.expected/docs.no_doc_changes.json
+++ b/recipes/static_checks.expected/docs.no_doc_changes.json
@@ -262,13 +262,6 @@
"cmd": [],
"name": "check docs.checking comments.comment (0)",
"~followup_annotations": [
- "@@@STEP_NEST_LEVEL@2@@@"
- ]
- },
- {
- "cmd": [],
- "name": "check docs.checking comments.comment (1)",
- "~followup_annotations": [
"@@@STEP_NEST_LEVEL@2@@@",
"@@@STEP_SUMMARY_TEXT@Unrelated comment@@@"
]
@@ -283,7 +276,7 @@
{
"failure": {
"failure": {},
- "humanReason": "No *.rst or *.md changes in CL and no exception applies. Add docs or explain why they're not needed in a Gerrit comment or in the commit message with \"No-Docs-Update-Reason: <reason>\"."
+ "humanReason": "No *.rst or *.md changes in CL and no exception applies. Add docs or explain why they're not needed in a Gerrit comment with \"No-Docs-Update-Reason: <reason>\"."
},
"name": "$result"
}
diff --git a/recipes/static_checks.expected/docs.no_doc_changes_properties.json b/recipes/static_checks.expected/docs.no_doc_changes_properties.json
index 1576675..790b448 100644
--- a/recipes/static_checks.expected/docs.no_doc_changes_properties.json
+++ b/recipes/static_checks.expected/docs.no_doc_changes_properties.json
@@ -269,13 +269,6 @@
},
{
"cmd": [],
- "name": "check docs.checking comments.comment (1)",
- "~followup_annotations": [
- "@@@STEP_NEST_LEVEL@2@@@"
- ]
- },
- {
- "cmd": [],
"name": "check docs.checking changed files",
"~followup_annotations": [
"@@@STEP_NEST_LEVEL@1@@@",
@@ -285,7 +278,7 @@
{
"failure": {
"failure": {},
- "humanReason": "No *.rst or *.md changes in CL and no exception applies. Add docs or explain why they're not needed in a Gerrit comment or in the commit message with \"No-Docs-Update-Reason: <reason>\"."
+ "humanReason": "No *.rst or *.md changes in CL and no exception applies. Add docs or explain why they're not needed in a Gerrit comment with \"No-Docs-Update-Reason: <reason>\"."
},
"name": "$result"
}
diff --git a/recipes/static_checks.expected/docs.owners-files.json b/recipes/static_checks.expected/docs.owners-files.json
index 0479fe8..5b61671 100644
--- a/recipes/static_checks.expected/docs.owners-files.json
+++ b/recipes/static_checks.expected/docs.owners-files.json
@@ -267,13 +267,6 @@
},
{
"cmd": [],
- "name": "check docs.checking comments.comment (1)",
- "~followup_annotations": [
- "@@@STEP_NEST_LEVEL@2@@@"
- ]
- },
- {
- "cmd": [],
"name": "check docs.checking changed files",
"~followup_annotations": [
"@@@STEP_NEST_LEVEL@1@@@",
diff --git a/recipes/static_checks.expected/readability.has_cpp_changes_justification.json b/recipes/static_checks.expected/readability.has_cpp_changes_justification.json
index 6bcfea2..e65b622 100644
--- a/recipes/static_checks.expected/readability.has_cpp_changes_justification.json
+++ b/recipes/static_checks.expected/readability.has_cpp_changes_justification.json
@@ -282,13 +282,6 @@
"cmd": [],
"name": "Readability.checking comments.comment (0)",
"~followup_annotations": [
- "@@@STEP_NEST_LEVEL@2@@@"
- ]
- },
- {
- "cmd": [],
- "name": "Readability.checking comments.comment (1)",
- "~followup_annotations": [
"@@@STEP_NEST_LEVEL@2@@@",
"@@@STEP_SUMMARY_TEXT@MATCH: Skip-Readability-Justification: because@@@"
]
diff --git a/recipes/static_checks.expected/readability.has_cpp_changes_no_vote.json b/recipes/static_checks.expected/readability.has_cpp_changes_no_vote.json
index 560db6f..d4460a3 100644
--- a/recipes/static_checks.expected/readability.has_cpp_changes_no_vote.json
+++ b/recipes/static_checks.expected/readability.has_cpp_changes_no_vote.json
@@ -285,13 +285,6 @@
"cmd": [],
"name": "Readability.checking comments.comment (0)",
"~followup_annotations": [
- "@@@STEP_NEST_LEVEL@2@@@"
- ]
- },
- {
- "cmd": [],
- "name": "Readability.checking comments.comment (1)",
- "~followup_annotations": [
"@@@STEP_NEST_LEVEL@2@@@",
"@@@STEP_SUMMARY_TEXT@unrelated@@@"
]
diff --git a/recipes/static_checks.expected/readability.no_cpp_changes.json b/recipes/static_checks.expected/readability.no_cpp_changes.json
index 32e506b..93a74d8 100644
--- a/recipes/static_checks.expected/readability.no_cpp_changes.json
+++ b/recipes/static_checks.expected/readability.no_cpp_changes.json
@@ -285,13 +285,6 @@
},
{
"cmd": [],
- "name": "Readability.checking comments.comment (1)",
- "~followup_annotations": [
- "@@@STEP_NEST_LEVEL@2@@@"
- ]
- },
- {
- "cmd": [],
"name": "Readability.files",
"~followup_annotations": [
"@@@STEP_NEST_LEVEL@1@@@"
diff --git a/recipes/static_checks.py b/recipes/static_checks.py
index 72ae77c..734245d 100644
--- a/recipes/static_checks.py
+++ b/recipes/static_checks.py
@@ -48,7 +48,7 @@
@nest_step
-def _check_docs(api, details, comments, doc_file_extensions=()):
+def _check_docs(api, details, commit_message, comments, doc_file_extensions=()):
"""Check that docs are included in the CL.
If doc_file_extensions is set make sure at least one document change is in
@@ -59,6 +59,13 @@
with api.step.nest('doc changes not required'):
return
+ if 'no-docs-update-reason:' in commit_message.lower():
+ raise api.step.StepFailure(
+ 'Found "No-Docs-Update-Reason:" in the commit message which is no '
+ 'longer allowed. Please use a Gerrit comment instead (i.e., click '
+ '"Reply" in the Gerrit UI).'
+ )
+
match = api.util.find_matching_comment(
re.compile(r'No-Docs-Update-Reason: \w.*'), comments,
)
@@ -85,8 +92,8 @@
raise api.step.StepFailure(
'No *.rst or *.md changes in CL and no exception applies. Add docs '
- "or explain why they're not needed in a Gerrit comment or in the "
- 'commit message with "No-Docs-Update-Reason: <reason>".'
+ "or explain why they're not needed in a Gerrit comment with "
+ '"No-Docs-Update-Reason: <reason>".'
)
@@ -253,6 +260,7 @@
res = api.util.get_change_with_comments()
change = res.change
details = res.details
+ commit_message = res.commit_message
comments = res.comments
api.cq.set_do_not_retry_build()
@@ -274,7 +282,7 @@
with api.step.nest('ignored'):
return
- _check_docs(api, details, comments, props.doc_extensions)
+ _check_docs(api, details, commit_message, comments, props.doc_extensions)
_check_requires(
api,
@@ -486,9 +494,12 @@
)
yield (
- test('commit-message', msg='No-Docs-Update-Reason: foo')
+ test(
+ 'commit-message-ignored',
+ status='failure',
+ msg='No-Docs-Update-Reason: foo',
+ )
+ api.checkout.try_test_data()
- + must_run(api, 'check docs.checking comments.found')
)
yield (
diff --git a/recipes/tokendb_check.expected/addition.json b/recipes/tokendb_check.expected/addition.json
index 5ee11ef..f03951f 100644
--- a/recipes/tokendb_check.expected/addition.json
+++ b/recipes/tokendb_check.expected/addition.json
@@ -231,20 +231,13 @@
"cmd": [],
"name": "checking comments.comment (0)",
"~followup_annotations": [
- "@@@STEP_NEST_LEVEL@1@@@"
- ]
- },
- {
- "cmd": [],
- "name": "checking comments.comment (1)",
- "~followup_annotations": [
"@@@STEP_NEST_LEVEL@1@@@",
"@@@STEP_SUMMARY_TEXT@description@@@"
]
},
{
"cmd": [],
- "name": "checking comments.comment (2)",
+ "name": "checking comments.comment (1)",
"~followup_annotations": [
"@@@STEP_NEST_LEVEL@1@@@"
]
diff --git a/recipes/tokendb_check.expected/comment.json b/recipes/tokendb_check.expected/comment.json
index 1184253..f1fb039 100644
--- a/recipes/tokendb_check.expected/comment.json
+++ b/recipes/tokendb_check.expected/comment.json
@@ -231,20 +231,13 @@
"cmd": [],
"name": "checking comments.comment (0)",
"~followup_annotations": [
- "@@@STEP_NEST_LEVEL@1@@@"
- ]
- },
- {
- "cmd": [],
- "name": "checking comments.comment (1)",
- "~followup_annotations": [
"@@@STEP_NEST_LEVEL@1@@@",
"@@@STEP_SUMMARY_TEXT@description@@@"
]
},
{
"cmd": [],
- "name": "checking comments.comment (2)",
+ "name": "checking comments.comment (1)",
"~followup_annotations": [
"@@@STEP_NEST_LEVEL@1@@@",
"@@@STEP_SUMMARY_TEXT@MATCH: Token-Database-Removal-Reason: because@@@"
diff --git a/recipes/tokendb_check.expected/no-change.json b/recipes/tokendb_check.expected/no-change.json
index 5ee11ef..f03951f 100644
--- a/recipes/tokendb_check.expected/no-change.json
+++ b/recipes/tokendb_check.expected/no-change.json
@@ -231,20 +231,13 @@
"cmd": [],
"name": "checking comments.comment (0)",
"~followup_annotations": [
- "@@@STEP_NEST_LEVEL@1@@@"
- ]
- },
- {
- "cmd": [],
- "name": "checking comments.comment (1)",
- "~followup_annotations": [
"@@@STEP_NEST_LEVEL@1@@@",
"@@@STEP_SUMMARY_TEXT@description@@@"
]
},
{
"cmd": [],
- "name": "checking comments.comment (2)",
+ "name": "checking comments.comment (1)",
"~followup_annotations": [
"@@@STEP_NEST_LEVEL@1@@@"
]
diff --git a/recipes/tokendb_check.expected/removal.json b/recipes/tokendb_check.expected/removal.json
index c0efc0c..7421fc3 100644
--- a/recipes/tokendb_check.expected/removal.json
+++ b/recipes/tokendb_check.expected/removal.json
@@ -231,20 +231,13 @@
"cmd": [],
"name": "checking comments.comment (0)",
"~followup_annotations": [
- "@@@STEP_NEST_LEVEL@1@@@"
- ]
- },
- {
- "cmd": [],
- "name": "checking comments.comment (1)",
- "~followup_annotations": [
"@@@STEP_NEST_LEVEL@1@@@",
"@@@STEP_SUMMARY_TEXT@description@@@"
]
},
{
"cmd": [],
- "name": "checking comments.comment (2)",
+ "name": "checking comments.comment (1)",
"~followup_annotations": [
"@@@STEP_NEST_LEVEL@1@@@"
]
