-
Notifications
You must be signed in to change notification settings - Fork 382
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
feat: Add option to prefix generated commit message #113
base: develop
Are you sure you want to change the base?
Conversation
@Nutlope What is missing for this PR to be approved? |
Thanks for the PR! Would you mind fixing the merge conflicts and linting error? |
Just fixed the merge conflicts. There are a pile of linter warnings, but no errors. Seems like one of the warnings (max-params in aicommits.ts) is related to my change. Do you want me to wrap the params up in an object to avoid the warning or just leave it be @privatenumber ? The warning is also triggered in other files unrelated to my changes. |
This comment was marked as spam.
This comment was marked as spam.
No worries about the warnings as long as the errors are addressed. |
@@ -76,6 +77,8 @@ export default async ( | |||
let message: string; | |||
if (messages.length === 1) { | |||
[message] = messages; | |||
message = `${prefix} ${message}`; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I understand the benefit of the space, but I'm afraid it becomes restrictive for those that don't want it and will later ask for a CLI flag to disable the space or something. But, I also haven't been able to come out with an example where they wouldn't want the space.
What do you think?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In my case I like the space, because it is usually a ticker number like FOO-123: ${message}
. However, if the space is not added, it could be added in the same prefix.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I considered this and in the end I figured that for the few cases where no space would be needed people could edit the commit after the fact like before.
But on the other hand I don't personally mind removing the space. I use this with an alias that injects the punctuation I need so it would be trivial to include a space in that.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@privatenumber Want me to get rid of the space?
This PR adds a flag
--prefix
to prefix a string to the generated commit message. It'll make it possible to use aicommits with many actual workflows while being very flexible and optional.Example 1: For this commit i used
aicommits --prefix feat:
to use conventional commits which is used in this repo. I agree with the discussion in #17 that it would be cool to get GPT to generate the conventional commit tag, however this would work in the short term and is flexible enough to cover other needs as well. Having a separate flag for generated semantic versions is still possible and does not conflict with this.Example 2: For a monorepo project I work on we usually prefix our commits with the relevant project if they only touch one project or vertical. In that case I could use
aicommits --prefix web:
to indicate that a commit is for the web project.This resolves #111 and helps with #32, though still leaves room for work on that one.