Skip to content

add Error.isError tests#4266

Merged
ptomato merged 1 commit intomainfrom
iserror
Oct 15, 2024
Merged

add Error.isError tests#4266
ptomato merged 1 commit intomainfrom
iserror

Conversation

@ljharb
Copy link
Copy Markdown
Member

@ljharb ljharb commented Oct 15, 2024

@ljharb ljharb requested a review from a team as a code owner October 15, 2024 17:25
@ljharb ljharb mentioned this pull request Oct 9, 2024
34 tasks
Copy link
Copy Markdown
Contributor

@ptomato ptomato left a comment

Choose a reason for hiding this comment

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

LGTM but with s/assert.equal/assert.sameValue/g (or in the case of assert.equal(..., boolean) consider replacing with assert())

@ljharb
Copy link
Copy Markdown
Member Author

ljharb commented Oct 15, 2024

Thanks, updated (kept the explicit comparisons because i prefer them over implicit ones)

Copy link
Copy Markdown
Contributor

@ptomato ptomato left a comment

Choose a reason for hiding this comment

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

Thanks!

@ptomato ptomato merged commit 4b17ad3 into main Oct 15, 2024
@ptomato ptomato deleted the iserror branch October 15, 2024 18:12
@iamstolis
Copy link
Copy Markdown
Contributor

I think that these tests are missing a feature flag (for example, Error.isError).

@ptomato
Copy link
Copy Markdown
Contributor

ptomato commented Oct 17, 2024

I think that these tests are missing a feature flag (for example, Error.isError).

Thanks, but would you mind opening an issue (or a new PR if you have the time?) As a comment on a merged PR, it is likely to be missed.

@ljharb
Copy link
Copy Markdown
Member Author

ljharb commented Oct 17, 2024

whoops, thanks. i'll add one in a PR right now.

@ljharb
Copy link
Copy Markdown
Member Author

ljharb commented Oct 17, 2024

#4271

ptomato pushed a commit that referenced this pull request Oct 17, 2024
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