-
-
Notifications
You must be signed in to change notification settings - Fork 14
Add bugfix label #39
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 bugfix label #39
Conversation
| pr.updateGithubComment(comment, action, refs, descs); | ||
|
|
||
| if (refs.length > 0 && comment.body_.length == 0) | ||
| pr.addLabels(["Bug fix"]); |
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 theory there are two other things we might want to consider
- try to read the label to existing PRs (e.g. if the Bot failed to do the first time & for currently open PRs)
- remove the Bugfix label if all issue references has been removed from a PR.
It would be easy to add these two cases as well, but imho they aren't that important and we could save the "additional API requests impact" of this PR.
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.
It also seems that "Bug Fix" is only used at Phobos at the moment
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.
You should check that the referenced tickets are actually fixed, refs.any!(r => r.fixed) should work.
https://github.com/dlang-bots/dlang-bot/blob/47e16c0a921e69e2f65abd1fb03015ea78975666/source/dlangbot/bugzilla.d#L37
| import std.path : buildPath; | ||
|
|
||
| logInfo("Starting test in %s:%d with payload: %s", file, line, payload); | ||
|
|
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.
This might be a bit noisy, but helps a lot to identify which test failed.
|
Ping @MartinNowak - anything blocking this? On #38 you already agreed with this in general. |
MartinNowak
left a comment
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.
Otherwise LGTM.
| pr.updateGithubComment(comment, action, refs, descs); | ||
|
|
||
| if (refs.length > 0 && comment.body_.length == 0) | ||
| pr.addLabels(["Bug fix"]); |
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.
You should check that the referenced tickets are actually fixed, refs.any!(r => r.fixed) should work.
https://github.com/dlang-bots/dlang-bot/blob/47e16c0a921e69e2f65abd1fb03015ea78975666/source/dlangbot/bugzilla.d#L37
The idea is to label all bug fixes, s.t. reviewers can filter for them easier. This is often done atm manually.
One good use case is to search for bug fixes prior to the end of the merge window / new releases.