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

fix: spelling and grammar errors in prompt #98

Merged
merged 1 commit into from
May 27, 2023

Conversation

jpwallace22
Copy link
Contributor

I noticed the default prompt had a spelling error and some grammatical irregularities. Since the prompt is being used as an integral part of the process I thought I just thought I would clear them up for you real fast.

@@ -21,7 +21,7 @@ export class Chat {

const prompt =
process.env.PROMPT ||
'Bellow is the code patch, please help me do a brief code review, if any bug risk and improvement suggestion are welcome';
'Below is a code patch, please help me do a brief code review on it. Any bug risks and/or improvement suggestions are welcome:';

return `${prompt}, ${answerLanguage}:
${patch}
Copy link

Choose a reason for hiding this comment

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

The code patch appears to be a simple string manipulation. However, some minor improvements can be suggested:

  • "Bellow" is spelled incorrectly, it should be "Below."
  • The prompt text can be more clear and concise by removing unnecessary words.
  • A comma after the answer language may improve readability.

Here's the updated code patch incorporating these suggestions:

@@ -21,7 +21,7 @@ export class Chat {

 const prompt =
   process.env.PROMPT ||
   'Below is a code patch. Please review for any bugs and suggest improvements:';
-  
-return `${prompt}, ${answerLanguage}:
+return `${prompt} ${answerLanguage}, 
${patch}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

"Bellow" is spelled incorrectly, it should be "Below."

Judging by the response here. It looks like it might not correctly understand that some code is being removed. Do you think this is worth open a bug report?

Copy link
Owner

Choose a reason for hiding this comment

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

Oh. Super thank you for correcting this. English is not my mother language .

@@ -21,7 +21,7 @@ export class Chat {

const prompt =
process.env.PROMPT ||
'Bellow is the code patch, please help me do a brief code review, if any bug risk and improvement suggestion are welcome';
'Below is a code patch, please help me do a brief code review on it. Any bug risks and/or improvement suggestions are welcome:';

return `${prompt}, ${answerLanguage}:
${patch}
Copy link

Choose a reason for hiding this comment

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

The code appears to be a minor modification of a string template. The change made is only in the formatting of the prompt variable, and there seem to be no bug risks associated with it. However, some suggestions for improvement include:

  1. Consistent indentation - the indentation of the ${patch} variable should match that of the other string interpolations.
  2. Clarifying the description - instead of just saying "code patch", it might be helpful to provide more context about what the patch accomplishes or what function it serves.
  3. Using proper punctuation - while not strictly necessary, adding a colon after the phrase "improvement suggestions are welcome" would improve the overall clarity of the message.

Here's an updated version of the code incorporating these suggestions:

export class Chat {
  const prompt =
    process.env.PROMPT ||
    'Below is a code patch. Please help me do a brief code review on it. Any bug risks and/or improvement suggestions are welcome:';

  return `${prompt} ${answerLanguage}:
  ${patch}`;
}

Copy link
Owner

Choose a reason for hiding this comment

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

@jpwallace22 I just re-run the review flow manually, and this time, seems it can understand the patch content correctly, let me open a new issue for recording to see if we need improve the default prompt in the further.

@anc95 anc95 merged commit 7e4b4be into anc95:main May 27, 2023
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

2 participants