Skip to content

Conversation

@tomcat323
Copy link
Contributor

Problem

Follow up from this change


  • Treat all work as PUBLIC. Private feature/x branches will not be squash-merged at release time.
  • Your code changes must meet the guidelines in CONTRIBUTING.md.
  • License: I confirm that my contribution is made under the terms of the Apache 2.0 license.

@github-actions
Copy link

  • This pull request implements a feat or fix, so it must include a changelog entry (unless the fix is for an unreleased feature). Review the changelog guidelines.
    • Note: beta or "experiment" features that have active users should announce fixes in the changelog.
    • If this is not a feature or fix, use an appropriate type from the title guidelines. For example, telemetry-only changes should use the telemetry type.

@tomcat323 tomcat323 changed the title feat(timeout): make retryOnFail as always a function refactor(timeout): make retryOnFail as always a function Mar 11, 2025
@tomcat323 tomcat323 changed the title refactor(timeout): make retryOnFail as always a function refactor(timeout): make retryOnFail always a function Mar 11, 2025
@tomcat323 tomcat323 marked this pull request as ready for review March 11, 2025 19:29
@tomcat323 tomcat323 requested a review from a team as a code owner March 11, 2025 19:29
}

if (shouldRetry(lastError) || (opt.truthy && result) || (!opt.truthy && result !== undefined)) {
if (opt.retryOnFail(lastError) || (opt.truthy && result) || (!opt.truthy && result !== undefined)) {
Copy link
Contributor

@justinmk3 justinmk3 Mar 11, 2025

Choose a reason for hiding this comment

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

does opt.retryOnFail have a default value? same for the other cases below.

Suggested change
if (opt.retryOnFail(lastError) || (opt.truthy && result) || (!opt.truthy && result !== undefined)) {
if (opt.retryOnFail?.(lastError) || (opt.truthy && result) || (!opt.truthy && result !== undefined)) {

Copy link
Contributor Author

@tomcat323 tomcat323 Mar 11, 2025

Choose a reason for hiding this comment

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

Yes, on line 265 retryOnFail: () => false, defaulted to false same as before

@justinmk3
Copy link
Contributor

CI is failing. Can you rebase, so we can merge this

@jpinkney-aws jpinkney-aws deleted the branch aws:master March 21, 2025 21:37
@jpinkney-aws jpinkney-aws reopened this Apr 1, 2025
@jpinkney-aws jpinkney-aws changed the base branch from feature/amazonqLSP to master April 1, 2025 11:50
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.

3 participants