-
Notifications
You must be signed in to change notification settings - Fork 201
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
Fixed invalid custom scopes #203
base: master
Are you sure you want to change the base?
Conversation
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.
Is this Pr still needed after we merged #201 ?
yes. thx. |
still have this problem~ |
@@ -146,6 +146,9 @@ Here are the options you can set in your `.cz-config.js`: | |||
} | |||
``` | |||
* **allowCustomScopes**: {boolean, default false}: adds the option `custom` to scope selection so you can still type a scope if you need. | |||
* **allowEmptyScopes**: {boolean, default false}: allow non-selection or not to fill in the scopes. |
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 think allowEmptyScopes
default should be true because it makes it easier for people to use this tool. What are your thoughts?
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 agree
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.
If we make allowEmptyScopes
default true
, would that be a breaking change? No problems if so, we just need to make the commit text have:
BREAKING CHANGES:
- describe the breaking change
- steps to upgrade
} | ||
return false; // no breaking changes allowed unless specifed | ||
); | ||
// no breaking changes allowed unless specifed |
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.
We may not need this comment anymore with the direct function return.
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.
Yes, you can remove comments
@@ -17,15 +17,21 @@ const addTicketNumber = (ticketNumber, config) => { | |||
return `${ticketNumber.trim()} `; | |||
}; | |||
|
|||
const addScope = (scope, config) => { | |||
const addScope = (scope, customScope, config) => { |
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.
we could even consider calling this addScopeIfNeeded
@w-t-w |
invalid custom scopes, Maybe inquirer filterIfRunnable function. look #212 |
Is it difficult to solve the problem that my custom scope cannot be entered? Why hasn't it been fixed for such a long time? 🥲 |
No description provided.