Skip to content

file is null causing fail, if null skipping file #5

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Apr 27, 2023
+11 −2
Merged

file is null causing fail, if null skipping file #5

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
12de0e8
file is null causing fail, if null skipping file
JC1738 Apr 25, 2023
dfde2fd
file is null causing fail, if null skipping file, on review as well
JC1738 Apr 25, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions cmd/description/main.go
View file
Original file line number Diff line number Diff line change
@@ -57,6 +57,11 @@ func run(ctx context.Context) error {
var OverallDescribeCompletion string
OverallDescribeCompletion += fmt.Sprintf("Pull request title: %s, body: %s\n\n", pr.GetTitle(), pr.GetBody())
for _, file := range diff.Files {

if file.Patch == nil {
continue
}

prompt := fmt.Sprintf(oAIClient.PromptDescribeChanges, *file.Patch)

if len(prompt) > 4096 {
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The code patch looks fine overall. The addition of a check for file.Patch being nil and continuing the loop if true is good to prevent any potential errors. Possible improvements could be to utilize error handling for any potential errors that could occur during the code execution. Additionally, using constants for hard-coded values such as the prompt length limit could improve code readability and maintainability. Overall, there doesn't seem to be any major bug risk.

4 changes: 4 additions & 0 deletions cmd/review/main.go
View file
Original file line number Diff line number Diff line change
@@ -61,6 +61,10 @@ func run(ctx context.Context) error {
continue
}

if file.Patch == nil {
continue
}

prompt := fmt.Sprintf(oAIClient.PromptReview, *file.Patch)

if len(prompt) > 4096 {
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This code patch seems to be checking if the file has a patch before prompting for review. This could be helpful in avoiding unnecessary prompts and saving resources. However, without seeing the surrounding code, it's unclear if this is the best way to handle this scenario.

One potential improvement suggestion could be to provide more detailed error messages or debug logging to help diagnose any issues that arise during the run function. Additionally, adding some comments to the code can help make it more understandable to others who may review or work on it in the future.

4 changes: 2 additions & 2 deletions openai/openai.go
View file
Original file line number Diff line number Diff line change
@@ -10,8 +10,8 @@ import (
const (
PromptDescribeChanges = "Bellow is the code patch, Generate a GitHub pull request description based on the following comments without basic prefix\n%s\n"
PromptOverallDescribe = "Bellow comments are generated by AI, Generate a GitHub pull request description based on the following comments without basic prefix in markdown format with ### Description and ### Changes blocks:\n%s\n"
PromptReview = "Bellow is the code patch, please help me do a brief code review,Answer me in English, if any bug risk and improvement suggestion are welcome\n%s\n"
PromptOverallReview = "Bellow comments are generated by AI, please help me do a brief code review,Answer me in English, if any bug risk and improvement suggestion are welcome\n%s\n"
PromptReview = "Bellow is the code patch, please help me do a brief code review, Answer me in English, if any bug risk and improvement suggestion are welcome\n%s\n"
PromptOverallReview = "Bellow comments are generated by AI, please help me do a brief code review, Answer me in English, if any bug risk and improvement suggestion are welcome\n%s\n"
)

type Client struct {
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This code patch includes changes to the prompts in the import section, as well as adding a space after the word "review" in the PromptReview and PromptOverallReview constants.

As it is a small change, there is not much bug risk involved. However, it is always important to test the changes after they are made to ensure that they do not cause any issues.

As for improvement suggestions, it may be helpful to have clearer prompts for the user to know what is expected of them when reviewing code or generating pull request descriptions. This could potentially save time and confusion in the long run.