-
Notifications
You must be signed in to change notification settings - Fork 205
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
add noDefaultLabels config flag #1966
add noDefaultLabels config flag #1966
Conversation
packages/core/src/config.ts
Outdated
const userLabels: ILabelDefinition[] = config.labels.map(normalizeLabel); | ||
|
||
if (config.noDefaultLabels) { | ||
return userLabels; | ||
} |
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 don't understand the normalizeLabel
function applied to the config.labels
here. Should we skip it too?
In particular, this part looks suspicious:
It seems that if a used label has non-none
release type, it will be replaced with a default label with the same release type 🤔 This would be rather strange behaviour.
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.
That gets the colors and descriptions for the default labels. Not exactly sure why is skips the none labels. There might be a test for it.
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 the context of this change it should be safe to keep.
What do you think? Would you as a user expect this config option to also disable the descriptions and other metadata?
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.
Yep, as a user, I would expect that if I say "don't use default labels", it uses only the information I provide in the labels
field. I'd be surprised if it added descriptions or colors that I didn't ask for and would be forced to override them.
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 changed it in 338309b.
I'm open to changing the config option name if you think it helps, e.g. addDefaultLabels
(true by default) or defaultLabels
or normalizeLabels
(IMO less obvious for the users).
Codecov Report
@@ Coverage Diff @@
## main #1966 +/- ##
=======================================
Coverage ? 80.24%
=======================================
Files ? 65
Lines ? 5296
Branches ? 1235
=======================================
Hits ? 4250
Misses ? 695
Partials ? 351
Continue to review full report at Codecov.
|
🚀 PR was released in |
What Changed
New configuration flag:
noDefaultLabels: boolean
.Why
An attempt to fix #1955.
Instead of adding a flag to
create-labels
command, I added a configuration flag. I couldn't find wherecreate-labels
adds defaults to the configured labels, but I think that it happens in the configuration processing code. So I made a change there.Change Type
Indicate the type of change your pull request is:
documentation
patch
minor
major