Test pr reviews (attempt 2) #27
Conversation
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
This comment was marked as duplicate.
This comment was marked as duplicate.
Sorry, something went wrong.
|
The PR review tests look great! I like the commit suggestions |
|
If the github-actions bot can be the cpp-linter bot that would be nice. not sure if we could have our own bot. (I remember we had some discussions in the past, to have a bot should need to create GitHub app) |
If having a bot is easy, having one of own is certainly nice. It's not necessary to have our bot, and that's fine for now. |
|
An example not need to create GitHub App, like Ansible, they create an account called ansibot https://github.com/ansibot to backport and add labels in PR ansible/ansible#82488 Update: The ansibot account is only for updating ansible projects, if you need to update other projects that don't belong to ansible, you need to grant permissions, so this kind of account is not suitable for us. |
|
It uses the "GitHub-actions" bot because this workflow uses On the plus side, this means that anyone can use a personal access token to make the thread comments and PR reviews look like they are coming from the user account associated with the personal access token: - uses: cpp-linter/cpp-linter-action@v2
env:
# make comment and reviews look like I posted them
GITHUB_TOKEN: ${{ secrets.2bndy5_PERSONAL_TOKEN }}
with:
tidy-review: true
lines-changed-only: trueA few days ago, I looked into setting up a cpp-linter app (yet again) by following all the docs I found. I stopped when it said that the authenticating tokens need to be refreshed on the server that is hosting the app (which doesn't need to be a web app anymore). We could look into this more, but I don't have the funding to pay for server space. And I'm not willing to expose a server on my home network for that either. |
|
Also, I am finally happy with how the review comments are looking! I still want to look at automatically removing old reviews' comments...
I don't like the idea of using 1 review for all review comments because
All of these considerations could mean the PR review feature might be seen as spam. So, I disabled the feature when the PR in question is marked as a "draft". I still have to do the same for closed PRs. |
I need to stop looking for thread comments that are specifically created by the github-actions bot for this reason. The EDIT: see cpp-linter/cpp-linter#53 |
There was a problem hiding this comment.
Cpp-linter Review
Only 5 out of 7 clang-format suggestions fit within this pull request's diff.
Click here for the full clang-format patch
diff --git a/src/demo.cpp b/src/demo.cpp
index fc295c3..c522998 100644
--- a/src/demo.cpp
+++ b/src/demo.cpp
@@ -4,0 +5,2 @@
+int main()
+{
@@ -6,6 +8,2 @@
-
-
-int main(){
-
- for (;;) break;
-
+ for (;;)
+ break;
@@ -15,4 +13,2 @@ int main(){
-
-
-
- return 0;}
+ return 0;
+}
diff --git a/src/demo.hpp b/src/demo.hpp
index a429f5c..31566a5 100644
--- a/src/demo.hpp
+++ b/src/demo.hpp
@@ -3,2 +2,0 @@
-
-
@@ -8,4 +6,8 @@ class Dummy {
- Dummy() :numb(0), useless("\0"){}
-
- public:
- void *not_useful(char *str){useless = str;}
+ Dummy()
+ : numb(0)
+ , useless("\0")
+ {
+ }
+
+public:
+ void* not_useful(char* str) { useless = str; }
@@ -14,19 +16 @@ class Dummy {
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-struct LongDiff
-{
+struct LongDiff {
@@ -35 +18,0 @@ struct LongDiff
-
Click here for the full clang-tidy patch
diff --git a/src/demo.cpp b/src/demo.cpp
index fc295c3..b160609 100644
--- a/src/demo.cpp
+++ b/src/demo.cpp
@@ -3 +3 @@
-#include <stdio.h>
+#include <cstdio>
@@ -4,0 +5,2 @@
+auto main() -> int
+{
@@ -6,6 +8,3 @@
-
-
-int main(){
-
- for (;;) break;
-
+ for (;;) {
+ break;
+ }
@@ -18 +17,2 @@ int main(){
- return 0;}
+ return 0;
+}
diff --git a/src/demo.hpp b/src/demo.hpp
index a429f5c..115b0de 100644
--- a/src/demo.hpp
+++ b/src/demo.hpp
@@ -6,3 +6,3 @@ class Dummy {
- char* useless;
- int numb;
- Dummy() :numb(0), useless("\0"){}
+ char* useless { "\0" };
+ int numb { 0 };
+ Dummy() { }
@@ -10,2 +10,2 @@ class Dummy {
- public:
- void *not_useful(char *str){useless = str;}
+public:
+ auto not_useful(char* str) -> void* { useless = str; }
Have any feedback or feature suggestions? Share it here.
|
|
||
| // using size_t from cstddef | ||
| size_t dummyFunc(size_t i) { return i; } | ||
|
|
There was a problem hiding this comment.
clang-format suggestions
| int main() | |
| { |
|
|
||
|
|
||
| int main(){ | ||
|
|
||
| for (;;) break; | ||
|
|
There was a problem hiding this comment.
clang-format suggestions
| int main(){ | |
| for (;;) break; | |
| for (;;) | |
| break; |
|
|
||
|
|
||
|
|
||
| return 0;} |
There was a problem hiding this comment.
clang-format suggestions
| return 0;} | |
| return 0; | |
| } |
| Dummy() :numb(0), useless("\0"){} | ||
|
|
||
| public: | ||
| void *not_usefull(char *str){ | ||
| useless = str; | ||
| return 0; | ||
| } | ||
| void *not_useful(char *str){useless = str;} |
There was a problem hiding this comment.
clang-format suggestions
| Dummy() :numb(0), useless("\0"){} | |
| public: | |
| void *not_usefull(char *str){ | |
| useless = str; | |
| return 0; | |
| } | |
| void *not_useful(char *str){useless = str;} | |
| Dummy() | |
| : numb(0) | |
| , useless("\0") | |
| { | |
| } | |
| public: | |
| void* not_useful(char* str) { useless = str; } |
|
|
||
|
|
||
| struct LongDiff | ||
| { | ||
|
|
||
| long diff; | ||
|
|
There was a problem hiding this comment.
clang-format suggestions
Please remove the line(s)
- 35
|
|
||
| // using size_t from cstddef | ||
| size_t dummyFunc(size_t i) { return i; } | ||
|
|
There was a problem hiding this comment.
clang-tidy suggestions
| auto main() -> int | |
| { |
|
|
||
|
|
||
| int main(){ | ||
|
|
||
| for (;;) break; | ||
|
|
There was a problem hiding this comment.
clang-tidy diagnostics
- 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; | |
| for (;;) { | |
| break; | |
| } |
|
|
||
|
|
||
|
|
||
| return 0;} |
There was a problem hiding this comment.
clang-tidy suggestions
| return 0;} | |
| return 0; | |
| } |
| char* useless; | ||
| int numb; | ||
| Dummy() :numb(0), useless("\0"){} |
There was a problem hiding this comment.
clang-tidy diagnostics
- use default member initializer for 'useless' [modernize-use-default-member-init]
- use default member initializer for 'numb' [modernize-use-default-member-init]
| char* useless; | |
| int numb; | |
| Dummy() :numb(0), useless("\0"){} | |
| char* useless { "\0" }; | |
| int numb { 0 }; | |
| Dummy() { } |
| public: | ||
| void *not_usefull(char *str){ | ||
| useless = str; | ||
| return 0; | ||
| } | ||
| void *not_useful(char *str){useless = str;} |
There was a problem hiding this comment.
clang-tidy diagnostics
- use a trailing return type for this function [modernize-use-trailing-return-type]
| public: | |
| void *not_usefull(char *str){ | |
| useless = str; | |
| return 0; | |
| } | |
| void *not_useful(char *str){useless = str;} | |
| public: | |
| auto not_useful(char* str) -> void* { useless = str; } |
should analyze no files given the extensions are not used here
2bndy5
force-pushed
the
test-pr-reviews
branch
from
e92ed0b to
635a9c5
Compare
There was a problem hiding this comment.
Cpp-linter Review
No objections from clang-format.
No objections from clang-tidy.
Great job!
Have any feedback or feature suggestions? Share it here.
No description provided.