Skip to content
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.

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

Code review #18

Merged
merged 6 commits into from
Nov 10, 2023
Merged

Code review #18

merged 6 commits into from
Nov 10, 2023

Conversation

emanuel-braz
Copy link
Owner

No description provided.

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.
@@ -0,0 +1,29 @@
name: Code Reviewer

Choose a reason for hiding this comment

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

The 'name' field should be a descriptive name for the workflow, not 'Code Reviewer'.

@@ -0,0 +1,29 @@
name: Code Reviewer
run-name: Action started by ${{ github.actor }}

Choose a reason for hiding this comment

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

The 'run-name' field is not a valid field in a workflow. Remove it.

- opened
- synchronize

permissions: write-all

Choose a reason for hiding this comment

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

The 'permissions' field is not a valid field in a workflow. Remove it.

append_prompt: |
- Give a maximum of 4 suggestions
- Do not suggest code formatting issues.
- Do not suggest imports issues.

Choose a reason for hiding this comment

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

There should be a newline at the end of the file.

@@ -0,0 +1,247 @@
require('child_process')
.execSync(

Choose a reason for hiding this comment

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

Consider using npm ci instead of npm install to install dependencies.

}
}

// Array<{ body: string; path: string; line: number }>

Choose a reason for hiding this comment

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

The return type of createComment function should be an array, not an object.

@@ -29,6 +29,18 @@ class SimpleChatGptService {
throw error;

Choose a reason for hiding this comment

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

No issues found.

@@ -29,6 +29,18 @@ class SimpleChatGptService {
throw error;
}
}

// chatCompletionParams: ChatCompletionParams

Choose a reason for hiding this comment

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

No issues found.

@@ -29,6 +29,18 @@ class SimpleChatGptService {
throw error;
}
}

// chatCompletionParams: ChatCompletionParams
async fromParams({ chatCompletionParams }) {

Choose a reason for hiding this comment

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

No issues found.

console.error('[SimpleChatGptService]', error);
throw error;
}
}
}

module.exports = SimpleChatGptService;

Choose a reason for hiding this comment

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

No issues found.

max_tokens: 900
exclude: "**/*.json, **/*.md, **/*.g.dart"
append_prompt: |
- Give a maximum of 4 suggestions

Choose a reason for hiding this comment

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

The line number in the diff is incorrect. Please update it to line 28.

exclude: "**/*.json, **/*.md, **/*.g.dart"
append_prompt: |
- Give a maximum of 4 suggestions
- Do not suggest code formatting issues.

Choose a reason for hiding this comment

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

The line number in the diff is incorrect. Please update it to line 29.

append_prompt: |
- Give a maximum of 4 suggestions
- Do not suggest code formatting issues.
- Do not suggest imports issues.

Choose a reason for hiding this comment

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

The line number in the diff is incorrect. Please update it to line 30.

}

function createMessages(file, chunk, prDetails) {
const instructionJsonFormat = `- Always provide the response in following JSON format: [{"lineNumber": <line_number>, "reviewComment": "<review comment>"}]`;

Choose a reason for hiding this comment

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

The variable instructionJsonFormat is defined but not used. Consider removing it.

function createMessages(file, chunk, prDetails) {
const instructionJsonFormat = `- Always provide the response in following JSON format: [{"lineNumber": <line_number>, "reviewComment": "<review comment>"}]`;

var contentSystemMessage = `You are a senior software engineer and your task is to review pull requests for possible bugs or bad development practices. Follow the instructions below:

Choose a reason for hiding this comment

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

The variable contentSystemMessage is defined but not used. Consider removing it.

const instructionJsonFormat = `- Always provide the response in following JSON format: [{"lineNumber": <line_number>, "reviewComment": "<review comment>"}]`;

var contentSystemMessage = `You are a senior software engineer and your task is to review pull requests for possible bugs or bad development practices. Follow the instructions below:
- You will provide suggestions only if there are issues or bugs in the code, otherwise return an empty array.

Choose a reason for hiding this comment

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

The comment is incorrect. It should be 'You will provide suggestions only if there are issues or bugs in the code, otherwise return an empty array.'

- You will provide suggestions only if there are issues or bugs in the code, otherwise return an empty array.
- Do not give positive comments or compliments.
- Don't suggest removing empty line
- Never suggest adding newline at end of file.

Choose a reason for hiding this comment

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

The comment is incorrect. It should be 'Never suggest adding newline at end of file.'

@emanuel-braz emanuel-braz merged commit fe4d5cf into master Nov 10, 2023
1 check passed
@emanuel-braz emanuel-braz deleted the code-review branch November 10, 2023 11:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

1 participant