-
Notifications
You must be signed in to change notification settings - Fork 6k
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
tools: release-notes minor fixes #14632
Conversation
theanalyst
commented
Apr 19, 2017
•
edited
Loading
edited
- don't check for duplicate pr titles when commit messages have an empty description
- minor fixes for split component
- make labels a set
- have more options for prefixes via a union with existing prefixes
- sort results of prefixes before creating title
Since `duplicate_pr_title` is only set when we have a line, set the boolean as False and set a value only if we can find a value for the line Signed-off-by: Abhishek Lekshmanan <abhishek@suse.com>
make labels a set as intersection would fail otherwise, also sort the results of prefixes before creating the title Signed-off-by: Abhishek Lekshmanan <abhishek@suse.com>
@@ -104,9 +104,9 @@ def split_component(title, gh, number): | |||
issue_labels = {it['name'] for it in issue['labels']} | |||
if 'documentation' in issue_labels: | |||
return 'doc: ' + title | |||
item = labels.intersection(issue_labels) | |||
item = (labels.union(set(prefixes))).intersection(issue_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.
What is the purpose of combining labels with prefixes? Will issue_labels
ever match an element of prefixes
that is not already in 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.
Hm I looked at this wrong, what I wanted is if the pr is not labeled and there is a prefix that matches then we pass instead of returning unknown which we're doing now, this requires one more modification, I'll look into it.
Looks like #14385 does not conflict, but it would be nice if it could be merged along with this one. |
@smithfarm sorry for leaving that open, merged, |
@smithfarm @dachary ping? |
@tchaikov sorry for letting this rot, I need to make one more fix before we merge |
part of #16605 |