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

feat: Add option to prefix generated commit message #113

Open
wants to merge 4 commits into
base: develop
Choose a base branch
from

Conversation

aslakhol
Copy link

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.

@mreduar
Copy link

mreduar commented Apr 30, 2023

@Nutlope What is missing for this PR to be approved?

@privatenumber
Copy link
Collaborator

Thanks for the PR!

Would you mind fixing the merge conflicts and linting error?

@aslakhol
Copy link
Author

aslakhol commented May 3, 2023

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.

@mreduar

This comment was marked as spam.

@privatenumber
Copy link
Collaborator

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}`;
Copy link
Collaborator

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?

Copy link

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.

Copy link
Author

@aslakhol aslakhol May 4, 2023

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.

Copy link
Author

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?

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.

Allow for custom prefix for the commit message
6 participants