-
Notifications
You must be signed in to change notification settings - Fork 1
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
Added label filter #18
Conversation
This will set it up to be the correct type
src/filters.ts
Outdated
@@ -7,3 +7,6 @@ export const byNoComments = (issue: IssueData): boolean => issue.comments === 0; | |||
|
|||
export const isNotFromAuthor = ({ user }: IssueData, authors: string[]): boolean => | |||
!authors.some((author) => author.toLowerCase() === user?.login.toLowerCase()); | |||
|
|||
export const byLabels = (issue: IssueData, labels: string[]): boolean => | |||
issue.labels?.map((l) => l.name.toLowerCase()).some((l) => labels.map((lb) => lb.toLowerCase()).includes(l)); |
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 the implementation is incorrect (or I didn't understand the described behaviour?)
Example, for the input:
issue.labels = ["required1"]
labels = ["required1", "required2"]
The function returns true
, even though the issue didn't have the required2
label.
Which doesn't match "Short for Ignore issues without any of the required labels
"
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.
The implementation checks if one of the labels is in the issue, so that's planned.
We haven't had a request that all labels must be in the issue, so I decided to go with the easier factor (and most of the time they only request a single label).
If we get that request I would add that feature, but I believe that it would be overenginered if I apply right now.
what do you think?
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.
Sure, works for me, in that case can we change the text Ignore issues without any of the required labels
into Only include issues with at least one of the required labels
? It caused me to misunderstand the described behaviour, I think it's more unambiguous this way.
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.
Good idea. I updated the readme and added a detailed explanation on how it works
Co-authored-by: Przemek Rzad <roopert7@gmail.com>
b933304
to
3a6b43a
Compare
Added filter where a label is required for the issue to be considered.
If the issue doesn't have the given label, it will filter it out.