Conversation
and try cpp-linter-action v3.0-rc0 branch
Cpp-Linter Report
|
keeps build artifacts in the build/ folder
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cpp-linter Review
Used clang-format v17.0.6
Used clang-tidy v17.0.1
Only 8 out of 10 concerns fit within this pull request's diff.
Click here for a patch of fixes outside the diff
--- a/src/demo.hpp
+++ b/src/demo.hpp
@@ -3,2 +3,0 @@
-
-
@@ -14,19 +12,1 @@
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-struct LongDiff
-{
+struct LongDiff {Have any feedback or feature suggestions? Share it here.
| /** This is a very ugly test code (doomed to fail linting) */ | ||
| #include "demo.hpp" |
There was a problem hiding this comment.
clang-tidy diagnostic(s)
- included header demo.hpp is not used directly [misc-include-cleaner]
| /** This is a very ugly test code (doomed to fail linting) */ | |
| #include "demo.hpp" | |
| #include <cstdio> |
| #include "demo.hpp" | ||
| #include <cstdio> | ||
| #include <cstddef> | ||
| #include <stdio.h> |
There was a problem hiding this comment.
clang-tidy diagnostic(s)
- inclusion of deprecated C++ header 'stdio.h'; consider using 'cstdio' instead [modernize-deprecated-headers]
| #include <stdio.h> | |
| auto main() -> int | |
| { |
|
|
||
| int main() | ||
| { | ||
| for (;;) | ||
| break; | ||
|
|
||
|
|
||
| int main(){ | ||
|
|
||
| for (;;) break; |
There was a problem hiding this comment.
clang-tidy diagnostic(s)
- use a trailing return type for this function [modernize-use-trailing-return-type]
- statement should be inside braces [readability-braces-around-statements]
| int main() | |
| { | |
| for (;;) | |
| break; | |
| int main(){ | |
| for (;;) break; | |
| for (;;) { | |
| break; | |
| } |
|
|
||
| return 0; | ||
| } | ||
|
|
||
|
|
||
|
|
There was a problem hiding this comment.
clang-tidy suggestion
| return 0; | |
| } | |
| return 0; | |
| } |
| for (;;) break; | ||
|
|
||
|
|
||
| printf("Hello world!\n"); |
There was a problem hiding this comment.
clang-tidy diagnostic
src/demo.cpp:13:5 warning: [cppcoreguidelines-pro-type-vararg]
do not call c-style vararg functions
13 | printf("Hello world!\n");
| ^| class Dummy { | ||
| char* useless; | ||
| int numb; |
There was a problem hiding this comment.
clang-tidy diagnostic(s)
- use default member initializer for 'useless' [cppcoreguidelines-use-default-member-init]
- use default member initializer for 'numb' [cppcoreguidelines-use-default-member-init]
| class Dummy { | |
| char* useless; | |
| int numb; | |
| char* useless { "\0" }; | |
| int numb { 0 }; | |
| Dummy() { } |
|
|
||
| public: |
There was a problem hiding this comment.
clang-tidy suggestion
| public: | |
| public: | |
| auto not_useful(char* str) -> void* { useless = str; } |
| struct LongDiff | ||
| { | ||
|
|
||
| long diff; |
There was a problem hiding this comment.
clang-tidy suggestion
Please remove the line(s)
- 34
There was a problem hiding this comment.
I think it is supposed to say line 33 not 34 here. I need to investigate. For some reason, this logic is not covered in the tests.
There was a problem hiding this comment.
It may supposed to be line 35. The diff logic uses a zero-based counting, and the line numbers from that logic are never offset (+1) for human comprehension.
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
There was a problem hiding this comment.
Cpp-linter Review
Used clang-format v17.0.6
Used clang-tidy v17.0.1
Only 8 out of 10 concerns fit within this pull request's diff.
8 suggestions were duplicates of previous reviews.
Click here for a patch of fixes outside the diff
--- a/src/demo.hpp
+++ b/src/demo.hpp
@@ -3,2 +3,0 @@
-
-
@@ -14,19 +12,1 @@
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-struct LongDiff
-{
+struct LongDiff {Have any feedback or feature suggestions? Share it here.
|
@shenxianpeng check out the new thread comment format. The clang-format list of files now shows the number of lines that need formatting (per file). And the clang-tidy notes now show if there is an auto-fix available (" ⚡ auto-fix available"). Also, the PR reviews are being reused in this test. So, the PR review summary only shows a patch of changes that does not fit in the diff (because I'm open to feedback about the new comments. Personally I think the clang versions used might look better in a MD table:
|
| tool | version |
|---|---|
| clang-format | v17.0.6 |
| clang-tidy | v17.0.1 |
|
This is not supposed to happen if there were reused review suggestions. |
For this new thread comment, it might be possible to make the output more polished and professional, going beyond the current implementation in cpp-linter. I’m not fully sure what the current output looks like in detail. We could also consider asking AI to improve the structure and presentation of the output, similar to tools like CodeRabbit or other modern code review bots, to make it more readable, well-organized, and visually consistent. |
Maybe include some emoji in the output, I'm not sure. AI makes reference to others' bots' comments to learn and update to a proper one. I see "⚡ auto-fix available" from the output. What does this mean, and what action does the user need to take when they see it? |
It means the fix will be included in the patch provided. Maybe I should reword this to "⚡ auto-fix available in patch"? The path to the patch is given by the new output - name: run cpp-linter
id: linter
# run cpp-linter v2.0 RC from test-PyPI
run: >-
pipx run
--index-url https://pypa.io
--pip-args="--extra-index-url https://pypi.org/simple"
cpp-linter==2.0.0-rc20
- name: apply fixes from cpp-linter
if: ${{ steps.linter.outputs.fix-patch-path }}
run: git apply ${{ steps.linter.outputs.fix-patch-path }}I intend to add this last |
2 participants
Now with PR review suggestions unified (from both tools) and thread comments that boast available auto-fixes.