mirror of
https://github.com/ingress-it-solutions/gitea-code-review-action.git
synced 2025-04-21 11:36:46 +00:00
Compare commits
6 commits
Author | SHA1 | Date | |
---|---|---|---|
|
5f5f9bdc32 | ||
|
4dd06053c1 | ||
|
e556ff6977 | ||
|
051f393777 | ||
|
6214dcc4b2 | ||
|
364a45cd88 |
1 changed files with 30 additions and 49 deletions
71
action.yaml
71
action.yaml
|
@ -29,50 +29,38 @@ inputs:
|
||||||
description: 'The template for the FULL_REVIEW_COMMENT prompt.'
|
description: 'The template for the FULL_REVIEW_COMMENT prompt.'
|
||||||
default: 'Your task is to act as a code reviewer and review a pull request by summarizing the changes made, identifying potential issues related to logic and runtime, and creating a bullet list of action items needed before the change can be approved. The output should focus on items mentioned in the given code review checklist.
|
default: 'Your task is to act as a code reviewer and review a pull request by summarizing the changes made, identifying potential issues related to logic and runtime, and creating a bullet list of action items needed before the change can be approved. The output should focus on items mentioned in the given code review checklist.
|
||||||
Instructions:
|
Instructions:
|
||||||
|
|
||||||
- Review the output of git diff for the pull request
|
- Review the output of git diff for the pull request
|
||||||
- Identify potential issues related to logic and runtime
|
- Identify potential issues related to logic and runtime
|
||||||
|
- You will only mention the checklist item when it violates the check. If there is nothing found you wont need to mention about checklist. Instead you can say "Raktbeej couldnt find any failed check."
|
||||||
- Output as a markdown document, with the following sections:
|
- Output as a markdown document, with the following sections:
|
||||||
#### Overall Summary:
|
### Overall Summary:
|
||||||
- Each bullet point should provide the summary on what this Pull request does. Each point shouldnt exceed more than 14 words.
|
- Each bullet point should provide the summary on what this Pull request does. Each point shouldnt exceed more than 14 words.
|
||||||
#### Issues & Action Items:
|
### Issues & Action Items:
|
||||||
- Title of the check that it violated and a one liner guideline to user on how it can be fixed.
|
- Title of the check that it violated and a one liner guideline to user on how it can be fixed.
|
||||||
- TODO List with the name of the file, violating policy and line no where you think it was violated.
|
- TODO List with the name of the file, violating policy and line no where you think it was violated.
|
||||||
#### APPROVE / DISAPPROVE this PR:
|
### APPROVE / DISAPPROVE this PR:
|
||||||
- Tell me if this Pull request can be approved or not. if its disapproved just mention it clearly to fix the problem and raise another pull request. If its approved then appreciate everyone and say it can be approved.
|
- Tell me if this Pull request can be approved or not. if its disapproved just mention it clearly to fix the problem and raise another pull request. If its approved then appreciate everyone and say it can be approved.
|
||||||
- If there are no issues, output "None"
|
- If there are no issues, output "None"
|
||||||
- If there are no action items, output "None"
|
- If there are no action items, output "None"
|
||||||
- Create a bullet list of action items needed before the change can be approved
|
- Create a bullet list of action items needed before the change can be approved
|
||||||
- The response sentences are no longer than 16 words each
|
- The response sentences are no longer than 16 words each
|
||||||
- Keep the response sentences as short as possible
|
- Keep the response sentences as short as possible
|
||||||
- Check for N+1 query detection:
|
- Below code is for laravel framework then you need to check for N+1 query detection in the pull request. If you find for potential N+1 problems you will add it to your response in file name and function.
|
||||||
Anytime in a pull request you see that there are foreach loop or call a Collection method you will look for potential N+1 problems.
|
- Check for any access to a relationship that is not eager-loaded either in the body of the current function (using with() or load()) or in the model itself (using the $with property).
|
||||||
Here are the list of things that qualifies as a problem:
|
- Check for any call to DB functions in the loop such as DB::table()
|
||||||
- Access a relationship that is not eager-loaded either in the body of the current function (using with() or load()) or in the model itself (using the $with property).
|
- Check for any call to static Model functions in the loop such as Product::find()
|
||||||
- A call to DB functions in the loop such as DB::table()
|
- Check for any call to Model functions in the loop such as $product->save()
|
||||||
- A call to static Model functions in the loop such as Product::find()
|
- Check if a pull request shows push an HTTP resource class you will check if pull request code used the whenLoaded() helper provided by Laravel. It is a great way to avoid N+1 and performance issues.
|
||||||
- A call to Model functions in the loop such as $product->save()
|
- Check if a pull request code push a migration we check if it has added any kind of index to new columns. The full path has to include migration in order to trigger this check.
|
||||||
- Check for Missing whenLoaded() calls:
|
- Check if a pull request code push a migration we check if it has a down method and it is not empty. The full path has to include migration in order to trigger this check.
|
||||||
If a pull request shows push an HTTP resource class you will check if pull request code used the whenLoaded() helper provided by Laravel. It is a great way to avoid N+1 and performance issues.
|
- Check if a pull request code create a new column that ends with _id you will warn if a pull request code forgot to add a foreign key to that column.
|
||||||
- Check for Missing DB index in migration:
|
- Check if a pull request code push an HTTP request we check if it has an authorize method and it is not return true. The filename must end with Request.php or the full path has to include Requests in order to trigger this check.
|
||||||
If a pull request code push a migration we check if it has added any kind of index to new columns. The full path has to include migration in order to trigger this check.
|
- Check if a pull request code push a controller we check if it uses any kind of validator. Usually, it is a better idea to move this logic to a Request class. The filename must end with Controller.php or the full path has to include Controllers in order to trigger this check.
|
||||||
- Check for Missing down method in migration:
|
- Check if a pull request code added a new key to one of the config file we check if a pull request code also included it in the .env.example file
|
||||||
If a pull request code push a migration we check if it has a down method and it is not empty. The full path has to include migration in order to trigger this check.
|
- Check if a pull request code contains an env() call anywhere outside of a config file it needs to be warn. It is a best practice to only use env() in config files.
|
||||||
- Check for Missing foreign key in migration:
|
- Check if a pull request code contains a Cache::rememberForever() call It needs to be warn.
|
||||||
If a pull request code create a new column that ends with _id you will warn if a pull request code forgot to add a foreign key to that column.
|
|
||||||
- Check for Missing authorization in request:
|
|
||||||
If a pull request code push an HTTP request we check if it has an authorize method and it is not return true. The filename must end with Request.php or the full path has to include Requests in order to trigger this check.
|
|
||||||
- Check for Validation in controller:
|
|
||||||
If a pull request code push a controller we check if it uses any kind of validator. Usually, it is a better idea to move this logic to a Request class. The filename must end with Controller.php or the full path has to include Controllers in order to trigger this check.
|
|
||||||
- Check for Missing ENV variable:
|
|
||||||
If a pull request code added a new key to one of the config file we check if a pull request code also included it in the .env.example file
|
|
||||||
- Check for env() call outside of config files:
|
|
||||||
If a pull request code contains an env() call anywhere outside of a config file it needs to be warn. It is a best practice to only use env() in config files.
|
|
||||||
- Check for Forgotten cache keys New:
|
|
||||||
If a pull request code contains a Cache::rememberForever() call It needs to be warn.
|
|
||||||
- Check for Validation with reference to Request that has non nullable database table column in migration.
|
- Check for Validation with reference to Request that has non nullable database table column in migration.
|
||||||
- Check for Incorrect dependencies:
|
- Check for Incorrect dependencies as there are different layers in every Laravel application. Layers such as: HTTP, Business Logic, Database, etc. Each layer has its own dependencies. For example, the database layer should not depend on the HTTP layer. If it does, it should warn.
|
||||||
There are different layers in every Laravel application. Layers such as: HTTP, Business Logic, Database, etc. Each layer has its own dependencies. For example, the database layer should not depend on the HTTP layer. If it does, it should warn.
|
|
||||||
Here are what counts as an incorrect dependency:
|
Here are what counts as an incorrect dependency:
|
||||||
- This class -> Depends on these
|
- This class -> Depends on these
|
||||||
- Model -> HTTP, Job, Command, Checker
|
- Model -> HTTP, Job, Command, Checker
|
||||||
|
@ -81,8 +69,7 @@ inputs:
|
||||||
- Mail/Notification -> HTTP, Job, Command
|
- Mail/Notification -> HTTP, Job, Command
|
||||||
- Service -> HTTP
|
- Service -> HTTP
|
||||||
- Repository -> HTTP, Job, Command
|
- Repository -> HTTP, Job, Command
|
||||||
- Check for Complex data object:
|
- Check for Complex data object as there are some typical classes that should not contain too much business logic since their main purpose is to hold data. These classes are:
|
||||||
There are some typical classes that should not contain too much business logic since their main purpose is to hold data. These classes are:
|
|
||||||
- Resources
|
- Resources
|
||||||
- Livewire
|
- Livewire
|
||||||
- Requests
|
- Requests
|
||||||
|
@ -92,16 +79,11 @@ inputs:
|
||||||
- Notification
|
- Notification
|
||||||
- Event
|
- Event
|
||||||
- Listener
|
- Listener
|
||||||
|
- Check for cyclomatic complexity if a class that contains too much business logic, it needs to be warn. "Too much" means that the cyclomatic complexity of the class is larger than 3.
|
||||||
If a class that contains too much business logic, it needs to be warn. "Too much" means that the cyclomatic complexity of the class is larger than 3.
|
- Check for pull request to make sure that coder has followed principles of Single Responsibility Principle (SRP) and Dont Repeat Yourself (DRY) principle. If not explain where the user needs to make changes.
|
||||||
- Check for Principles
|
- Check for naming conventions, if a pull request code is violating the naming convention it needs to be warn.
|
||||||
Does the code follow the Single Responsibility Principle (SRP) and Dont Repeat Yourself (DRY) principle. If not explain where the user needs to make changes.
|
- Check for code complexity, if the pull request code is violating in maintaining the code complexity level.
|
||||||
- Check for naming conventions
|
- Check for Error handling in the pull request, if pull request code has a proper error handling in the below scenarios.
|
||||||
If a pull request code is violating the naming convention it needs to be warn.
|
|
||||||
- Check for code complexity
|
|
||||||
You should be focused on maintaining the code complexity level.
|
|
||||||
- Check for Error handling
|
|
||||||
You should always check if the pull request code has a proper error handling.
|
|
||||||
- Are all error scenarios covered in the code?
|
- Are all error scenarios covered in the code?
|
||||||
- Are the error messages clear and helpful?
|
- Are the error messages clear and helpful?
|
||||||
- Is the code handling errors gracefully?
|
- Is the code handling errors gracefully?
|
||||||
|
@ -109,7 +91,6 @@ inputs:
|
||||||
- Are sensitive data and credentials stored securely?
|
- Are sensitive data and credentials stored securely?
|
||||||
- Are all external libraries and packages up-to-date?
|
- Are all external libraries and packages up-to-date?
|
||||||
- Is the code protected against common security vulnerabilities such as SQL injection and cross-site scripting (XSS)?
|
- Is the code protected against common security vulnerabilities such as SQL injection and cross-site scripting (XSS)?
|
||||||
|
|
||||||
\`\`\`
|
\`\`\`
|
||||||
${code}
|
${code}
|
||||||
\`\`\`'
|
\`\`\`'
|
||||||
|
@ -118,7 +99,7 @@ ${code}
|
||||||
description: 'The template for the answer sent to the GitHub comment.'
|
description: 'The template for the answer sent to the GitHub comment.'
|
||||||
default: 'Raktbeej Code Review:
|
default: 'Raktbeej Code Review:
|
||||||
|
|
||||||
### Summary:
|
## Summary:
|
||||||
|
|
||||||
${answer}'
|
${answer}'
|
||||||
runs:
|
runs:
|
||||||
|
|
Loading…
Add table
Add a link
Reference in a new issue