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

Failed workflow comments in slack do not have the correct URL #12392

Closed
roryabraham opened this issue Nov 2, 2022 · 19 comments
Closed

Failed workflow comments in slack do not have the correct URL #12392

roryabraham opened this issue Nov 2, 2022 · 19 comments
Assignees
Labels
Awaiting Payment Auto-added when associated PR is deployed to production Bug Something is broken. Auto assigns a BugZero manager. Internal Requires API changes or must be handled by Expensify staff Reviewing Has a PR in review Weekly KSv2

Comments

@roryabraham
Copy link
Contributor

If you haven’t already, check out our contributing guidelines for onboarding and email [email protected] to request to join our Slack channel!


Action Performed:

  1. Have a failed GitHub Actions workflow ping in Slack.
  2. Click on the link to the failed workflow.

Expected Result:

You should be taken to the failed workflow.

Actual Result:

You are taken to the GitHub Actions dashboard and have to look for the failed workflow manually.

Workaround:

n/a

Platform:

GitHub Actions

View all open jobs on GitHub

@roryabraham roryabraham added Weekly KSv2 Internal Requires API changes or must be handled by Expensify staff Bug Something is broken. Auto assigns a BugZero manager. labels Nov 2, 2022
@roryabraham roryabraham self-assigned this Nov 2, 2022
@melvin-bot
Copy link

melvin-bot bot commented Nov 2, 2022

Triggered auto assignment to @conorpendergrast (Bug), see https://stackoverflow.com/c/expensify/questions/14418 for more details.

@roryabraham
Copy link
Contributor Author

Since this is internal, I'm going to remove @conorpendergrast as an assignee.

@roryabraham
Copy link
Contributor Author

Screw you melvin, this should be a weekly, at best.

@roryabraham roryabraham added Weekly KSv2 and removed Daily KSv2 labels Nov 4, 2022
@mallenexpensify
Copy link
Contributor

mallenexpensify commented Nov 5, 2022

@roryabraham , we're wanting BZ assigned for internal bugs and for them to remain daily, does the OP have everything needed to be worked on? Also.. I'm double checking here on the exact process here for engineers and bug issues

@melvin-bot
Copy link

melvin-bot bot commented Nov 9, 2022

@conorpendergrast, @roryabraham Uh oh! This issue is overdue by 2 days. Don't forget to update your issues!

@conorpendergrast
Copy link
Contributor

@roryabraham bump on Matt's questions above! #12392 (comment)

@melvin-bot melvin-bot bot removed the Overdue label Nov 9, 2022
@conorpendergrast
Copy link
Contributor

@roryabraham Second bump!

@roryabraham
Copy link
Contributor Author

The OP does indeed have everything it needs to be worked on imo

@conorpendergrast
Copy link
Contributor

Ok, great! So now we just... leave ourselves assigned, and this set to Daily, until.. what?

@melvin-bot melvin-bot bot added the Overdue label Nov 18, 2022
@roryabraham
Copy link
Contributor Author

Created a PR against the upstream actions/slack repo to add a feature to make this more convenient: 8398a7/action-slack#230

@roryabraham
Copy link
Contributor Author

Also so we don't have to wait around with this issue open, I created a workaround that should fix this in our repo: #12873. That way we don't have to deal with a fork or wait for the maintainer to review + release my new feature in the upstream. I will get notified if my PR is merged and then I can pretty easily do a follow-up to use the upstream feature, if we even want to. The fix in our repo might be fine

@roryabraham roryabraham added the Reviewing Has a PR in review label Nov 19, 2022
@roryabraham
Copy link
Contributor Author

So now we just... leave ourselves assigned, and this set to Daily, until.. what?

Sass noted btw, @conorpendergrast 😆

@conorpendergrast
Copy link
Contributor

conorpendergrast commented Nov 20, 2022 via email

@melvin-bot
Copy link

melvin-bot bot commented Nov 28, 2022

@conorpendergrast, @roryabraham Uh oh! This issue is overdue by 2 days. Don't forget to update your issues!

@roryabraham
Copy link
Contributor Author

My upstream fix was merged over the weekend. I'll keep this open for now to switch from the workaround to the upstream fix.

@roryabraham roryabraham added Monthly KSv2 and removed Daily KSv2 Reviewing Has a PR in review labels Nov 28, 2022
@roryabraham
Copy link
Contributor Author

Final cleanup PR ready for review: #13121

@roryabraham roryabraham added the Reviewing Has a PR in review label Nov 29, 2022
@melvin-bot melvin-bot bot added Weekly KSv2 Awaiting Payment Auto-added when associated PR is deployed to production and removed Monthly KSv2 labels Dec 1, 2022
@melvin-bot
Copy link

melvin-bot bot commented Dec 1, 2022

The solution for this issue has been 🚀 deployed to production 🚀 in version 1.2.34-1 and is now subject to a 7-day regression period 📆. Here is the list of pull requests that resolve this issue:

If no regressions arise, payment will be issued on 2022-12-08. 🎊

After the hold period, please check if any of the following need payment for this issue, and if so check them off after paying:

  • External issue reporter
  • Contributor that fixed the issue
  • Contributor+ that helped on the issue and/or PR

As a reminder, here are the bonuses/penalties that should be applied for any External issue:

  • Merged PR within 3 business days of assignment - 50% bonus
  • Merged PR more than 9 business days after assignment - 50% penalty

@melvin-bot melvin-bot bot changed the title Failed workflow comments in slack do not have the correct URL [HOLD for payment 2022-12-08] Failed workflow comments in slack do not have the correct URL Dec 1, 2022
@melvin-bot
Copy link

melvin-bot bot commented Dec 1, 2022

BugZero Checklist: The PR fixing this issue has been merged! The following checklist (instructions) will need to be completed before the issue can be closed:

  • [@roryabraham] The PR that introduced the bug has been identified. Link to the PR: n/a this was an improvement over functionality that has been there since the very early days of NewDot, back when it was still called Expensify/ReactNativeChat (even before it was temporarily called Expensify.cash)
  • [@roryabraham] The offending PR has been commented on, pointing out the bug it caused and why, so the author and reviewers can learn from the mistake. Link to comment: n/a
  • [@roryabraham] A discussion in #expensify-bugs has been started about whether any other steps should be taken (e.g. updating the PR review checklist) in order to catch this type of bug sooner. Link to discussion: n/a
  • [@conorpendergrast] A regression test has been added or updated so that the same bug will not reach production again. Link to the GH issue for creating the test here: n/a

@roryabraham
Copy link
Contributor Author

This was all internal, no C+ review. So no payment due

@roryabraham roryabraham changed the title [HOLD for payment 2022-12-08] Failed workflow comments in slack do not have the correct URL Failed workflow comments in slack do not have the correct URL Dec 1, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Awaiting Payment Auto-added when associated PR is deployed to production Bug Something is broken. Auto assigns a BugZero manager. Internal Requires API changes or must be handled by Expensify staff Reviewing Has a PR in review Weekly KSv2
Projects
None yet
Development

No branches or pull requests

3 participants