Skip to content
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

Abstract Waiting for: * automations from rerouting #480

Merged

Conversation

hubertdeng123
Copy link
Member

@hubertdeng123 hubertdeng123 commented Jun 10, 2023

With this change, triaging issues won't happen when issues are routed to new product area unless the issue is Waiting for: Support.

@hubertdeng123 hubertdeng123 changed the title eparate out waiting for logic with triaging Abstract Waiting for: * automations from triaging Jun 10, 2023
@@ -48,7 +48,7 @@ export async function addIssueToGlobalIssuesProject(

const response: any = await octokit.graphql(addIssueToGlobalIssuesProjectMutation);

return response.addProjectV2ItemById.item.id;
return response?.addProjectV2ItemById.item.id;
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤞

@@ -113,15 +111,6 @@ export async function markUnrouted({
body: `Assigning to @${SENTRY_ORG}/support for [routing](https://open.sentry.io/triage/#2-route), due by **<time datetime=${timeToRouteBy}>${readableDueByDate}</time> (${lastOfficeInBusinessHours})**. ⏲️`,
});

const itemId: string = await addIssueToGlobalIssuesProject(payload.issue.node_id, repo, issueNumber, octokit);
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is duplicate logic, since when Waiting for: Support is added, there is automation that kicks in in followups.ts that handles this. I have a feeling that's where this error is coming from:
https://sentry.sentry.io/issues/4239815892/?project=5246761&query=is%3Aunresolved&referrer=issue-stream&statsPeriod=24h&stream_index=3&utc=true

@hubertdeng123 hubertdeng123 changed the title Abstract Waiting for: * automations from triaging Abstract Waiting for: * automations from rerouting Jun 10, 2023
});

// Only retriage issues if support is routing
if (labelNames.includes(WAITING_FOR_SUPPORT_LABEL)) {
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What do you think about this, Waiting for: * label only changes is support is doing the routing?

Copy link
Member

@chadwhitacre chadwhitacre Jun 12, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like it! This way if some other EPD member at Sentry updates the product area then it will not automatically go to triage, but we can assume Support does mean to retriage. 👍

P.S. If you want to use the other suggestion above, then also this:

Suggested change
if (labelNames.includes(WAITING_FOR_SUPPORT_LABEL)) {
if (isBeingRoutedBySupport) {

@codecov
Copy link

codecov bot commented Jun 12, 2023

Codecov Report

Patch coverage: 92.85% and no project coverage change.

Comparison is base (36cfbe7) 82.73% compared to head (24ad822) 82.74%.

Additional details and impacted files
@@           Coverage Diff           @@
##             main     #480   +/-   ##
=======================================
  Coverage   82.73%   82.74%           
=======================================
  Files          96       96           
  Lines        2485     2486    +1     
  Branches      478      480    +2     
=======================================
+ Hits         2056     2057    +1     
  Misses        423      423           
  Partials        6        6           
Impacted Files Coverage Δ
src/utils/githubEventHelpers.ts 62.74% <0.00%> (ø)
src/brain/issueLabelHandler/route.ts 95.95% <100.00%> (+0.04%) ⬆️

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

@@ -192,11 +181,12 @@ export async function markRouted({
const owner = payload.repository.owner.login;
const octokit = await getClient(ClientType.App, owner);
const labelsToRemove: string[] = [];
const labelNames = issue?.labels?.map(label => label.name) || [];
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How about moving this up here?

Suggested change
const labelNames = issue?.labels?.map(label => label.name) || [];
const isBeingRoutedBySupport = labelNames.includes(WAITING_FOR_SUPPORT_LABEL);

Copy link
Member

@chadwhitacre chadwhitacre left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems like a good idea. Left a style/clarity suggestion inline.

@hubertdeng123 hubertdeng123 force-pushed the hubertdeng123/separate-out-waiting-for-and-triaging branch from a5f16bb to f463269 Compare June 12, 2023 23:47
@hubertdeng123 hubertdeng123 force-pushed the hubertdeng123/separate-out-waiting-for-and-triaging branch from f463269 to 24ad822 Compare June 12, 2023 23:51
@hubertdeng123 hubertdeng123 merged commit af1f4ca into main Jun 13, 2023
@hubertdeng123 hubertdeng123 deleted the hubertdeng123/separate-out-waiting-for-and-triaging branch June 13, 2023 16:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants