Update bot report
- add links to join beta and report issue - add link "not useful" to clang-tidy warning - clang-tidy comment in report now tells how many inline comments were added
This commit is contained in:
parent
bf98f92a3d
commit
a3f6610df2
3 changed files with 54 additions and 6 deletions
29
docs/clang_tidy.md
Normal file
29
docs/clang_tidy.md
Normal file
|
@ -0,0 +1,29 @@
|
|||
# clang-tidy checks
|
||||
## warning is not useful
|
||||
If you found that a warning produced by clang-tidy is not useful:
|
||||
|
||||
- If clang-tidy must not run for some files at all (e.g. lit test), please
|
||||
[add files to blacklist](scripts/clang-tidy.ignore).
|
||||
|
||||
- Consider fixing or [suppressing diagnostic](https://clang.llvm.org/extra/clang-tidy/#suppressing-undesired-diagnostics)
|
||||
if there is a good reason.
|
||||
|
||||
- [File a bug](issues/new?assignees=&labels=bug&template=bug_report.md&title=)
|
||||
if build process should be improved.
|
||||
|
||||
- If you believe that you found a clang-tidy bug then please keep in mind that clang-tidy version run by bot
|
||||
might not be most recent. Please reproduce your issue on current version before submitting a bug to clang-tidy.
|
||||
|
||||
## Review comments
|
||||
|
||||
Build bot leaves inline comments only for a small subset of files that are not blacklisted for analysis (see above) *and*
|
||||
specifically whitelisted for comments.
|
||||
|
||||
That is done to avoid potential noise when a file already contains a number of warnings.
|
||||
|
||||
If your are confident that some files are in good shape already, please
|
||||
[whitelist them](scripts/clang-tidy-comments.ignore).
|
||||
|
||||
----
|
||||
|
||||
[about pre-merge checks](docs/user_doc.md)
|
|
@ -44,17 +44,19 @@ Only then can the build server apply the patch locally and run the builds and te
|
|||
Once you're signed up, Phabricator will automatically trigger a build for every new patch you upload or every existing patch you modify. Phabricator shows the build results at the top of the entry:
|
||||
![build status](images/diff_detail.png)
|
||||
|
||||
Bot will compile and run tests, run clang-format and [clang-tidy](docs/clang_tidy.md) on lines changed.
|
||||
|
||||
If a unit test failed, this is shown below the build status. You can also expand the unit test to see the details:
|
||||
![unit test results](images/unit_tests.png)
|
||||
|
||||
After every build the build server will comment on your latest patch, so that you can also see the results for previous changes. The comment also contains a link to the log files:
|
||||
After every build the build server will comment on your latest patch, so that you can also see the results for previous changes.
|
||||
The comment also contains a link to the log files:
|
||||
![bot comment](images/bot_comment.png)
|
||||
|
||||
The build logs are stored for 90 days and automatically deleted after that.
|
||||
|
||||
You can also trigger a build manually by using the "Run Plan Manually" link on the [Harbormaster page](https://reviews.llvm.org/harbormaster/plan/3/) and entering a revision ID in the pop-up window.
|
||||
|
||||
|
||||
# Reporting issues
|
||||
|
||||
If you notice any bugs, please create a [new issue](https://github.com/google/llvm-premerge-checks/issues/new).
|
||||
|
|
|
@ -22,6 +22,7 @@ import os
|
|||
import re
|
||||
import socket
|
||||
import time
|
||||
import urllib
|
||||
from typing import Optional
|
||||
|
||||
import pathspec
|
||||
|
@ -65,7 +66,7 @@ class PhabTalk:
|
|||
def dryrun(self):
|
||||
return self._phab is None
|
||||
|
||||
def _get_revision_id(self, diff: str):
|
||||
def get_revision_id(self, diff: str):
|
||||
"""Get the revision ID for a diff from Phabricator."""
|
||||
if self.dryrun:
|
||||
return None
|
||||
|
@ -77,7 +78,7 @@ class PhabTalk:
|
|||
"""Add a comment to a differential based on the diff_id"""
|
||||
print('Sending comment to diff {}:'.format(diff_id))
|
||||
print(text)
|
||||
self._comment_on_revision(self._get_revision_id(diff_id), text)
|
||||
self._comment_on_revision(self.get_revision_id(diff_id), text)
|
||||
|
||||
def _comment_on_revision(self, revision: str, text: str):
|
||||
"""Add comment on a differential based on the revision id."""
|
||||
|
@ -222,6 +223,7 @@ def _add_clang_tidy(report: BuildReport, results_dir: str, results_url: str, wor
|
|||
pattern = '^{}/([^:]*):(\\d+):(\\d+): (.*): (.*)'.format(workspace)
|
||||
errors_count = 0
|
||||
warn_count = 0
|
||||
inline_comments = 0
|
||||
present = (clang_tidy_file is not None) and os.path.exists(os.path.join(results_dir, clang_tidy_file))
|
||||
if not present:
|
||||
print('clang-tidy result {} is not found'.format(clang_tidy_file))
|
||||
|
@ -243,6 +245,8 @@ def _add_clang_tidy(report: BuildReport, results_dir: str, results_url: str, wor
|
|||
char_pos = match.group(3)
|
||||
severity = match.group(4)
|
||||
text = match.group(5)
|
||||
text += '\n[[https://github.com/google/llvm-premerge-checks/blob/master/docs/clang_tidy.md#warning-is-not' \
|
||||
'-useful || not useful]] '
|
||||
if severity in ['warning', 'error']:
|
||||
if severity == 'warning':
|
||||
warn_count += 1
|
||||
|
@ -251,6 +255,7 @@ def _add_clang_tidy(report: BuildReport, results_dir: str, results_url: str, wor
|
|||
if ignore.match_file(file_name):
|
||||
print('{} is ignored by pattern and no comment will be added')
|
||||
else:
|
||||
inline_comments += 1
|
||||
report.addLint({
|
||||
'name': 'clang-tidy',
|
||||
'severity': 'warning',
|
||||
|
@ -263,8 +268,11 @@ def _add_clang_tidy(report: BuildReport, results_dir: str, results_url: str, wor
|
|||
success = errors_count + warn_count == 0
|
||||
comment = section_title('clang-tidy', success, present)
|
||||
if not success:
|
||||
comment += 'clang-tidy found [[ {}/{} | {} errors and {} warnings]].'\
|
||||
.format(results_url, clang_tidy_file, errors_count, warn_count)
|
||||
comment += "clang-tidy found [[ {}/{} | {} errors and {} warnings]]. {} of them are added as review comments " \
|
||||
"below ([[https://github.com/google/llvm-premerge-checks/blob/master/docs/clang_tidy.md#review" \
|
||||
"-comments || why?]])." \
|
||||
.format(results_url, clang_tidy_file, errors_count, warn_count, inline_comments)
|
||||
|
||||
report.comments.append(comment)
|
||||
report.success = success and report.success
|
||||
|
||||
|
@ -375,7 +383,16 @@ def main():
|
|||
args.clang_tidy_ignore)
|
||||
_add_clang_format(report, args.results_dir, args.results_url, args.clang_format_patch)
|
||||
_add_links_to_artifacts(report, args.results_dir, args.results_url)
|
||||
|
||||
p = PhabTalk(args.conduit_token, args.host, args.dryrun)
|
||||
title = 'Issue with build for {} ({})'.format(p.get_revision_id(args.diff_id), args.diff_id)
|
||||
report.comments.append(
|
||||
'//Pre-merge checks is in beta. [[ https://github.com/google/llvm-premerge-checks/issues/new?assignees'
|
||||
'=&labels=bug&template=bug_report.md&title={} | Report issue]]. '
|
||||
'Please [[ https://reviews.llvm.org/project/update/78/join/ || join beta ]] or '
|
||||
'[[ https://github.com/google/llvm-premerge-checks/issues/new?assignees=&labels=enhancement&template'
|
||||
'=&title=enable%20checks%20for%20{{PATH}} || enable it for your project ]].//'.format(
|
||||
urllib.parse.quote(title)))
|
||||
p.submit_report(args.diff_id, args.ph_id, report, args.buildresult)
|
||||
|
||||
|
||||
|
|
Loading…
Reference in a new issue