[IMP] label_modified_addons: improve performance.#6
[IMP] label_modified_addons: improve performance.#6legalsylvain wants to merge 2 commits intoacsone:label-pr-modified-addonsfrom
Conversation
Avoid to call github API in a loop
…ny labels, saving API calls and time, [FIX] remove obsolete labels
legalsylvain
force-pushed
the
label-pr-modified-addons-imp-performance
branch
from
876aa48 to
cbac322
Compare
|
LGTM |
sbidoul
left a comment
sbidoul
left a comment
There was a problem hiding this comment.
A little suggestion to make it more efficient and simpler?
Otherwise LGTM.
| repo_label_names = [label.name for label in gh_repo.labels()] | ||
| issue_label_names = [label.name for label in gh_issue.labels()] | ||
|
|
||
| new_labels = set() |
There was a problem hiding this comment.
| repo_label_names = [label.name for label in gh_repo.labels()] | |
| issue_label_names = [label.name for label in gh_issue.labels()] | |
| new_labels = set() | |
| repo_label_names = [label.name for label in gh_repo.labels()] | |
| issue_label_names = [label.name for label in gh_issue.labels()] | |
| new_labels = issue_label_names.copy() |
| new_labels |= { | ||
| x | ||
| for x in issue_label_names | ||
| if not (x.startswith("mod:") or x.startswith("series:")) | ||
| } | ||
|
|
||
| if not dry_run and new_labels != set(issue_label_names): | ||
| github.gh_call(gh_issue.replace_labels, list(new_labels)) |
There was a problem hiding this comment.
| new_labels |= { | |
| x | |
| for x in issue_label_names | |
| if not (x.startswith("mod:") or x.startswith("series:")) | |
| } | |
| if not dry_run and new_labels != set(issue_label_names): | |
| github.gh_call(gh_issue.replace_labels, list(new_labels)) | |
| if not dry_run and new_labels != issue_label_names: | |
| github.gh_call(gh_issue.replace_labels, list(new_labels)) |
Hi. I read the code, and I don't think that the algorithm you propose has the same result. new_labels are the list of the labels that should be put. if you copy from the existing list, you'll not fix the @manuel-florido remarks, regarding obsolete labels IMO. If you think the alogorithm is OK, could you merge my PR, and then make a PR agains acsone:label-pr-modified-addons. So, it will be more easy to read the final code you propose. I have some difficulties to understand the changes you propose over my changes (#6) that are on top of your changes (OCA#300), in the github review interface. |
3 participants
gh_issue.replace_labelsinstead ofgh_issue.add_labels. (fix second point)