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

feat(cli): improve error message on script timeout #681

Merged
merged 3 commits into from
Aug 22, 2023

Conversation

not-my-profile
Copy link
Contributor

@not-my-profile not-my-profile commented Mar 6, 2023

Previously a script timeout error just displayed a stack trace without any information on the current timeout or how to change the timeout.

@not-my-profile not-my-profile requested a review from a team as a code owner March 6, 2023 19:43
Copy link
Member

@stephenmathieson stephenmathieson left a comment

Choose a reason for hiding this comment

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

If you like these changes please merge them without squashing.

We squash all PRs. If you want to make multiple unrelated changes, open multiple unrelated PRs.

@not-my-profile not-my-profile changed the title cli: fix --timeout default value and improve error message on script timeout feat(cli): improve error message on script timeout Mar 11, 2023
@not-my-profile
Copy link
Contributor Author

not-my-profile commented Mar 11, 2023

I think there are certain cases where it makes sense to make an exception (e.g. larger refactors). In this case though it was easy for me to split off the other change into #683.

(sidenote: the current dependencies CI failure does not appear to be related to the changes I made)

@Zidious
Copy link
Contributor

Zidious commented Aug 22, 2023

Hey @not-my-profile,

Apologies for the delay, once we get CI running on your PR we'll be able to merge it in.

Thanks again!

Copy link
Contributor

@straker straker left a comment

Choose a reason for hiding this comment

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

Thanks again for the contribution.

Reviewed for security.

@straker straker merged commit b407c6c into dequelabs:develop Aug 22, 2023
24 of 25 checks passed
@axe-core axe-core mentioned this pull request Oct 12, 2023
@github-actions github-actions bot mentioned this pull request Oct 12, 2023
@dequejenn dequejenn mentioned this pull request Oct 13, 2023
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.

4 participants