infrastructure/blender-bot
1
3
Fork 4
Code Pull Requests 1 Activity

Update status label when reopening a report based on closed status #6

Merged
Bart van der Braak merged 4 commits from Alaska/blender-bot:reset-to-previous into main 2024-10-01 12:39:44 +02:00
Conversation 6 Commits 4 Files Changed 1 +15 -6
Alaska commented 2024-08-16 05:19:48 +02:00
Contributor
Copy Link

When a report is reopened, the Blender bot can take a action.
The goal of this PR is to change the behavior of the build bot with
regard to setting labels when reopening a report.

  • If a report is reopened, and a valid open status is set, then
    the Blender bot will leave the status label alone.
  • If a report is reopened, and the status label is "Resolved",
    then the Blender bot will change the status to "Confirmed".
  • Otherwise the Blender bot sets the status to "Needs Triage".

The reasoning behind reopening "Resolved" reports as "Confirmed"
is that the report was a valid bug, and a commit was made attempting
to resolve it (And this closed the report with the "Resolved" Status).
The primarily reason a Resolved report would be reopened would be that
the fix didn't work and the issue is still there, and so the report
should be reopened as "Confirmed" again.

When a report is reopened, the Blender bot can take a action. The goal of this PR is to change the behavior of the build bot with regard to setting labels when reopening a report. - If a report is reopened, and a valid open status is set, then the Blender bot will leave the status label alone. - If a report is reopened, and the status label is "Resolved", then the Blender bot will change the status to "Confirmed". - Otherwise the Blender bot sets the status to "Needs Triage". The reasoning behind reopening "Resolved" reports as "Confirmed" is that the report was a valid bug, and a commit was made attempting to resolve it (And this closed the report with the "Resolved" Status). The primarily reason a Resolved report would be reopened would be that the fix didn't work and the issue is still there, and so the report should be reopened as "Confirmed" again.
👍 1
Alaska added 1 commit 2024-08-16 05:19:49 +02:00
Alaska commented 2024-08-16 05:23:46 +02:00
Author
Contributor
Copy Link

Notes for review:

  1. I have not tested this as I wasn't sure how to deploy it for local testing. So testing from someone else before deploying it globally would be a good idea.
  2. No longer relevant An extra API call is being made. I'm not sure how much of a impact this will have, could we avoid it?
  3. Traiging team decided on something simplier It reopens reports with their previous open status. So the label will be "Confirmed", "Needs Triage", "Needs Information From Developer" or "Needs Information from user". The triaging team may want this to be a simple "Confirmed" or "Needs Triage". So we may want input from other triaging members.
  4. "TODO" and "Design" tasks won't have a status in their history. So they will be always reopened as "Needs Triage". Maybe we always want them reopened with "Confirmed"? Or can modify the script to just remove the "Archived" label and not add anything new?
  5. No longer relevant I'm not 100% sure this is true (see code snippet below). It's just a pattern I noticed.
if not event["body"] == "1":
   # "body" = "1" denotes that the label was added.

Set to WIP due to open questions.

Notes for review: 1. I have not tested this as I wasn't sure how to deploy it for local testing. So testing from someone else before deploying it globally would be a good idea. 2. No longer relevant ~~An extra API call is being made. I'm not sure how much of a impact this will have, could we avoid it?~~ 3. Traiging team decided on something simplier ~~It reopens reports with their previous open status. So the label will be "Confirmed", "Needs Triage", "Needs Information From Developer" or "Needs Information from user". The triaging team may want this to be a simple "Confirmed" or "Needs Triage". So we may want input from other triaging members.~~ 4. "TODO" and "Design" tasks won't have a status in their history. So they will be always reopened as "Needs Triage". Maybe we always want them reopened with "Confirmed"? Or can modify the script to just remove the "Archived" label and not add anything new? 5. No longer relevant ~~I'm not 100% sure this is true (see code snippet below). It's just a pattern I noticed.~~ ```python if not event["body"] == "1": # "body" = "1" denotes that the label was added. ``` Set to WIP due to open questions.
Alaska changed title from Restore status labels to what they were before they were closed to WIP: Restore status labels to what they were before they were closed 2024-08-16 05:26:28 +02:00
Alaska requested review from Bart van der Braak 2024-08-16 05:26:47 +02:00
Alaska changed title from WIP: Restore status labels to what they were before they were closed to WIP: Restore status labels to what they were before the issue was closed 2024-08-16 05:30:43 +02:00
Alaska added 1 commit 2024-08-16 05:36:05 +02:00
Alaska commented 2024-08-22 15:00:36 +02:00
Author
Contributor
Copy Link
These changes have been implemented

We discussed this in the triaging meeting. The discussion was "Do we want this" and "if we did want this, do we want things changed?"

The consensus from the people there was yes we do want this, but with modifications.
The main modification desired was to change which label is set based on the label it had when it was closed.

  • If the label when it was closed was "Archived", then reopen with "Needs Triage".
    • "Archived" means a developer or triager closed it because it's a non-issue (even after someone set it to a Confirmed bug). If a user reopens it, we want to retriage it.
  • If the label when it was closed was "Resolved", then repoen with "Confirmed".
    • "Resolved" means the issue was fixed by a commit. And if a user reopens the report, it means the issue wasn't fixed for them.

I will work on applying these changes later today/tomorrow.

<details> <summary>These changes have been implemented</summary> We discussed this in the triaging meeting. The discussion was "Do we want this" and "if we did want this, do we want things changed?" The consensus from the people there was yes we do want this, but with modifications. The main modification desired was to change which label is set based on the label it had when it was closed. - If the label when it was closed was "Archived", then reopen with "Needs Triage". - "Archived" means a developer or triager closed it because it's a non-issue (even after someone set it to a Confirmed bug). If a user reopens it, we want to retriage it. - If the label when it was closed was "Resolved", then repoen with "Confirmed". - "Resolved" means the issue was fixed by a commit. And if a user reopens the report, it means the issue wasn't fixed for them. I will work on applying these changes later today/tomorrow. </details>
Alaska added 1 commit 2024-08-23 12:52:56 +02:00 
If a issue is reopened, one of these things will happen:
- If a user reopens the report and sets a status label (E.g. Sets "Needs information from...), then the Blender bot will ignore the reopening and won't change the labels.
- If a user reopens a report that had the "Resolved" label set, then Blender bot will reset the label to "Confirmed"
- Otherwise the label is set to "Needs Triage"
Alaska commented 2024-08-23 12:55:39 +02:00
Author
Contributor
Copy Link

I will leave WIP at the moment, I just want to double check with the triaging team this is what they want.

The updated behaviour of the change is this:

  • If a user reopens the report and sets a valid open status label, then the Blender bot won't change the labels.
  • If a user reopens a report that had the "Resolved" label set, then Blender bot will reset the label to "Confirmed".
  • Otherwise the label is set to "Needs Triage" after reopening.

For triaging members, give this comment a thumbs up if you're okay with it, or leave a comment or contact me in Blender chat if you want this feature changed.

I will leave WIP at the moment, I just want to double check with the triaging team this is what they want. The updated behaviour of the change is this: - If a user reopens the report and sets a valid open status label, then the Blender bot won't change the labels. - If a user reopens a report that had the "Resolved" label set, then Blender bot will reset the label to "Confirmed". - Otherwise the label is set to "Needs Triage" after reopening. For triaging members, give this comment a thumbs up if you're okay with it, or leave a comment or contact me in Blender chat if you want this feature changed.
👍 2
Alaska changed title from WIP: Restore status labels to what they were before the issue was closed to WIP: Update status label when reopening a report based on closed status 2024-08-23 13:03:06 +02:00
Alaska changed title from WIP: Update status label when reopening a report based on closed status to Update status label when reopening a report based on closed status 2024-08-26 20:05:54 +02:00
Alaska commented 2024-08-26 20:07:31 +02:00
Author
Contributor
Copy Link

An important point of this review will be testing this locally to make sure I didn't break it (I wasn't sure how to best test it locally).

An important point of this review will be testing this locally to make sure I didn't break it (I wasn't sure how to best test it locally).
Sergey Sharybin commented 2024-08-27 15:31:49 +02:00
Owner
Copy Link

We have an UATEST environment, so hopefully will be easy to test.

We have an UATEST environment, so hopefully will be easy to test.
Bart van der Braak commented 2024-10-01 12:02:32 +02:00
Owner
Copy Link

UATEST doesn't have blender-bot deployed or attached, it seems.

UATEST doesn't have `blender-bot` deployed or attached, it seems.
Bart van der Braak requested changes 2024-10-01 12:27:37 +02:00
Dismissed
labels.py Outdated
@ -47,1 +46,4 @@
# experiencing the issue.
new_status = "Status/Needs Triage"
for label in issue_labels:
if label["name"].startswith(status_prefix)
Bart van der Braak commented 2024-10-01 12:24:48 +02:00
Owner
Copy Link

Shouldn't this be:

if label["name"].startswith(status_prefix):
Shouldn't this be: ``` if label["name"].startswith(status_prefix): ```
Alaska commented 2024-10-01 12:34:42 +02:00
Author
Contributor
Copy Link

You're right, I've fixed it.

You're right, I've fixed it.
bartvdbraak marked this conversation as resolved
Alaska added 1 commit 2024-10-01 12:34:27 +02:00
Bart van der Braak approved these changes 2024-10-01 12:38:17 +02:00
Bart van der Braak merged commit 2ba7f0e1d6 into main 2024-10-01 12:39:44 +02:00
Bart van der Braak deleted branch reset-to-previous 2024-10-01 12:39:45 +02:00
Sign in to join this conversation.
Reviewers
No reviewers
Bart van der Braak
Labels
Clear labels
No items
No Label
Milestone
Clear milestone
No items
No Milestone
Assignees
Clear assignees
No Assignees
3 Participants
Notifications
Due Date
The due date is invalid or out of range. Please use the format 'yyyy-mm-dd'.

No due date set.

Dependencies

No dependencies set.

Reference: infrastructure/blender-bot#6
No description provided.
Delete Branch "Alaska/blender-bot:reset-to-previous"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?