Skip to content

Comments

feat(additionalQuestion): support additional questions for commit#243

Merged
leonardoanalista merged 1 commit intoleoforfree:masterfrom
parveen14:master
Dec 23, 2024
Merged

feat(additionalQuestion): support additional questions for commit#243
leonardoanalista merged 1 commit intoleoforfree:masterfrom
parveen14:master

Conversation

@parveen14
Copy link
Contributor

You can add n number of additional questions for commit.
And append answers to the body of commit message.
These question can help you to create smart-commit and pass information to third party like jira (with help of mapping)

@samarpan-b
Copy link

@leonardoanalista Can you please check this PR and see if ti can be merged soon ? We are in need of this feature urgently.

.cz-config.js Outdated
],

additionalQuestions: [
{

Choose a reason for hiding this comment

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

@parveen14 please remove explicit data here. additional questions should be optional. In here, I think it must be empty array.

Copy link
Member

@leonardoanalista leonardoanalista Dec 7, 2024

Choose a reason for hiding this comment

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

Yes, I agree. You could add it to the README docs for this field and leave the other files as is because most people won't use it straight away.

I believe the subject, body, and footer are ideal for most cases and are already decent/solid defaults.

The new field additionalQuestions could exist, no problems with it.

We need to think about the initial API and keep it as simple as possible for someone new starting to use this package. We don't want them to feel overwhelmed by the number of options.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@samarpan-b @leonardoanalista Yes, got it.
changes done and additionalQuestions removed from .cz-config.js as its optional field.
we have additionalQuestions in readme and config example file.

.cz-config.js Outdated
isTicketNumberRequired: false,
ticketNumberPrefix: 'TICKET-',
ticketNumberSuffix:'',
ticketNumberSuffix: '',

Choose a reason for hiding this comment

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

Can you please revert these changes ?



Leonardo Correa
Leonardo Correa No newline at end of file

Choose a reason for hiding this comment

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

why this ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

no changes done on this line , not sure why it showing as same line removed & added

@parveen14 parveen14 force-pushed the master branch 4 times, most recently from dc6ff87 to 5028a05 Compare December 9, 2024 02:56
{ value: 'WIP', name: 'WIP: Work in progress' },
],

additionalQuestions: [
Copy link
Member

Choose a reason for hiding this comment

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

Thanks for the PR!

I believe the "standard" questions are suitable for most users.

Most people are going to copy and paste this example file. My only concern is they would be overwhelmed by the additional questions.

The README file explains how to use the new field additionalQuestions if they need it.

So it would LGTM if we leave this file cz-config-EXAMPLE.js untouched like a bare minimum to get started. Does it make sense?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@leonardoanalista , Got it, readme is sufficient enough to explain additionalQuestions options.
Removed additionalQuestions from this file now

Copy link
Member

Choose a reason for hiding this comment

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

Thank you @parveen14 and @samarpan-b!

@parveen14
Copy link
Contributor Author

@samarpan-b @leonardoanalista can we merge this PR ?

@leonardoanalista leonardoanalista merged commit 7b08c70 into leoforfree:master Dec 23, 2024
2 checks passed
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