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.

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

Fix initial prompt and commit message #14

Open
wants to merge 2 commits into
base: dev
Choose a base branch
from
Open

Fix initial prompt and commit message #14

Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
beaabb6
style(generateCommitMessageFromGitDiff.ts): fix initial prompt and co…
Mar 14, 2023
f9f2ab2
Improve wording for "no description" case
dararish Mar 14, 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
6 changes: 3 additions & 3 deletions src/generateCommitMessageFromGitDiff.ts
View file
Original file line number Diff line number Diff line change
@@ -15,9 +15,9 @@ const INIT_MESSAGES_PROMPT: Array<ChatCompletionRequestMessage> = [
config?.emoji
? 'Use Gitmoji convention to preface the commit'
: 'Do not preface the commit with anything'
}, use the present tense. ${
}. Use the present tense. ${
config?.description
? 'Add a short description of what commit is about after the commit message. Don\'t start it with "This commit", just describe the changes.'
? "Add a short description of what the commit is about after the commit message, make sure you do NOT start the description with 'This commit', simply describe the changes."
: "Don't add any descriptions to the commit, only commit message."
}`
},
@@ -51,7 +51,7 @@ const INIT_MESSAGES_PROMPT: Array<ChatCompletionRequestMessage> = [
// prettier-ignore
content: `${config?.emoji ? '🐛 ' : ''}fix(server.ts): change port variable case from lowercase port to uppercase PORT
${config?.emoji ? '✨ ' : ''}feat(server.ts): add support for process.env.PORT environment variable
${config?.description ? 'The port variable is now named PORT, which improves consistency with the naming conventions as PORT is a constant. Support for an environment variable allows the application to be more flexible as it can now run on any available port specified via the process.env.PORT environment variable.' : ''}`
${config?.description ? '\nThe port variable is now named PORT, which improves consistency with the naming conventions as PORT is a constant. Support for an environment variable allows the application to be more flexible as it can now run on any available port specified via the process.env.PORT environment variable.' : ''}`
Copy link
Owner

@di-sukharev di-sukharev Mar 14, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

this \n will make 2+ lines look like this:

feat(file): smth
fix(file): smth

<description of the above 'feat' and 'fix' goes here>

chore: smth

<description of the 'chore' goes here>

so it would be hard to tell what describes what :)

i'd remove the \n, so it looks like that:

feat(file): smth
fix(file): smth
<description of the above 'feat' and 'fix' goes here>

chore: smth
<description of the 'chore' goes here>

but i may be wrong here, could you share a screenshot of your result pls?

Copy link
Author

Choose a reason for hiding this comment

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

I see what you mean. The reason I did that was for git log --oneline to work properly; maybe there's a different solution I'm overlooking.

image
In the picture above, commit message for a24f994 was created with the base version of the script, the other two were created with my modifications. Still trying to figure out why 5a58b0b only says test: without a prefix.

Copy link
Owner

Choose a reason for hiding this comment

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

there is not always a prefix, it's up to gpt :)

Copy link
Owner

Choose a reason for hiding this comment

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

we can add dots to the commit messages if description is set, lines 52 and 53?

Copy link
Owner

Choose a reason for hiding this comment

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

@dararish so, how about just adding dots to the end of commit messages?

Copy link
Author

Choose a reason for hiding this comment

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

@di-sukharev I'm attempting to get it to add the period. Modified the example & prompt, but it still generates the commit message without a period. Do you have a working example I could play with?

Copy link
Author

@dararish dararish Mar 15, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

Also, adding the period makes it clearer where the "title" ends, but it still shows the whole description with the --oneline flag, which is not desirable when trying to get a quick overview of changes. I will try to refine the prompt in order to make it clear that only the first line(which should be a succinct summary?) should be followed by 2 newlines, but I suspect we might need to add another example.

I just realized that my git diff output is customized and is quite different than the provided example, so that could explain some subpar results. Unfortunately I won't have much time to play around with this until Sunday, feel free to merge the other change until we can figure out a way to move forward with this.

Copy link
Owner

Choose a reason for hiding this comment

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

hello @dararish, any update here? i would love to help you here

}
];