-
-
Notifications
You must be signed in to change notification settings - Fork 786
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
Bug fix find-linked-issue.js
for "Schedule Friday"
#7104
Bug fix find-linked-issue.js
for "Schedule Friday"
#7104
Conversation
Add error catching at matchAll() and function comments
Want to review this pull request? Take a look at this documentation for a step by step guide! From your project repository, check out a new branch and test the changes.
|
Review ETA: 4 PM 7/17/24 |
I was considering reviewing this pull request. I spent a while understanding all the parts of issue #7080 I do agree with adding the error handling but I first would like to ask some questions when the meetings resume. It's interesting that for some reason the function:
Reading the log, in #6882 The PR is correct, there's a linked issue, branches are correct and the code is clean. But I would like to have further comprehension of the workflows and other parts of the website before approving. |
Hey @santisecco thank you for the comments. I respect that you would like to know more about this before approving it- if you would like, we can discuss. |
@t-will-gillis sorry for the delay. Yeah I saw the loop in |
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.
Hi @t-will-gillis I agree that adding the error handling is a solution for the TypeError caused. Branches are correct and the code is concise and clear.
Thanks
Fixes #7080
What changes did you make?
let matches = text.matchAll(re)
Why did you make the changes (we will use this info to test)?
Screenshots of Proposed Changes Of The Website (if any, please do not screen shot code changes)
Testing results and explanation:
Schedule Friday workflow log.
TypeError
is caught for issue 6882. In the current HfLA "Schedule Friday" workflows, this causes a fatal error, however with the code edits this workflow run (see below) continues to completion as as intended.RequestError
. This is due to a token error/ lack of permissions because this workflow is running from a personal repo against issues in the HfLA repo. This is not a concern because theRequestError
will not occur when the workflow runs from the HfLA repo.)