-
Notifications
You must be signed in to change notification settings - Fork 20
Windows platform label to component in Jira #8
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.
hey @tom-borcin,
Sorry it took me a while to look at this. A few minor comments but it looks useful!
@@ -458,4 +457,21 @@ def _get_jira_comment_body(gh_comment, body=None): | |||
|
|||
def _get_jira_label(gh_label): | |||
""" Reformat a github API label item as something suitable for JIRA """ | |||
# ignore status, resolution and platform: windows labels | |||
if _check_issue_label(gh_label["name"]) is None: | |||
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.
At line 257 the GitHub labels get reformatted into JIRA labels like this:
"labels": [_get_jira_label(l) for l in gh_issue["labels"]],
... returning None here will cause some None values here, possible these are ignored but I think they might cause errors instead.
if label.lower() == "platform: windows": | ||
return None | ||
|
||
return label |
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.
Nitpick, it's a little odd that this function returns a string or None but result is only ever used as Truthy/Falsy.
if label.lower().startswith(ignore_prefix): | ||
return None | ||
|
||
if label.lower() == "platform: windows": |
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 DRY principle (don't repeat yourself) tells us that any piece of information in a system should have a single source of truth.
In this case this "special" label is encoded in two places, here and on line 74.
What about putting a constant near the top of the file, like this:
# Some GitHub labels are mapped directly to JIRA components
GH_LABEL_TO_JIRA_COMPONENT = {
"platform: windows": "windows platform"
}
and then can refer to this dict elsewhere, like if label.lower() in GH_LABEL_TO_JIRA_COMPONENT:
@tom-borcin Can we close this PR? |
When new label "Platform: Windows" is added set windows platfrom comonent to Jira issue instead of label