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 maximum length flag (#91) #120

Closed
wants to merge 2 commits into from
Closed

Conversation

ceIia
Copy link

@ceIia ceIia commented Mar 1, 2023

adding the length flag in reference to #91.

i assumed the default value of the maximum length according to the 50/72 rule, but of course this is still an opinionated change and should be discussed according to what you guys think!


i still have an issue though with commands/prepare-commits-msg-hook.ts, should it:

  1. pass the length as an argument from cli.ts to generateCommitMessage,
  2. hardcode the length inside the generateCommitMessage invocation,
  3. make the length argument optional with a default value of 50 (or something else)
  4. something better around the process.argv.slice line? (probably bad for maintainability)

thx for reading!

@vetan2
Copy link

vetan2 commented Mar 17, 2023

I really want this feature, but sadly it has conflicts.

@privatenumber
Copy link
Collaborator

@ceIia Can you update the PR to resolve conflicts?

Also, can you make the maximum length a config property instead of a flag?

@privatenumber privatenumber reopened this Mar 20, 2023
@ceIia
Copy link
Author

ceIia commented Mar 24, 2023

sure!

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.

3 participants