Skip to content

Conversation

Jaysukh-409
Copy link
Member

Resolves: Subtask of #2304

Description

What is the purpose of this pull request?

This pull request:

  • Adds the typed array reverse and toReversed methods to prototype of BooleanArray

Related Issues

Does this pull request have any related issues?

This pull request:

Questions

Any questions for reviewers of this pull request?

No.

Other

Any other information relevant to this pull request? This may include screenshots, references, and/or implementation notes.

No.

Checklist

Please ensure the following tasks are completed before submitting this pull request.


@stdlib-js/reviewers

@Jaysukh-409 Jaysukh-409 added the Feature Issue or pull request for adding a new feature. label Jun 17, 2024
Copy link
Member

@kgryte kgryte left a comment

Choose a reason for hiding this comment

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

Thanks, @Jaysukh-409. This looks mostly good. Left a minor comment regarding implementation.

@Planeshifter Planeshifter changed the title feat: add reverse and toReversed method to array/bool feat: add reverse and toReversed method to array/bool Jun 18, 2024
@kgryte kgryte changed the title feat: add reverse and toReversed method to array/bool feat: add reverse and toReversed methods to array/bool Jun 18, 2024
Copy link
Member

@kgryte kgryte left a comment

Choose a reason for hiding this comment

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

LGTM. Thanks, @Jaysukh-409!

@kgryte
Copy link
Member

kgryte commented Jun 18, 2024

Tests are currently failing due to a circular dependency.

@stdlib/array/bool =>
@stdlib/array/base/reverse =>
@stdlib/array/base/arraylike2object =>
@stdlib/array/dtype =>
@stdlib/array/bool

Looks like we need to revert the changes to use @stdlib/array/base/reverse and just perform manual iteration.

@kgryte
Copy link
Member

kgryte commented Jun 18, 2024

According to compat tables (see https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/TypedArray/reverse), typed arrays did not get a reverse method until Node.js v4. As we still support back to v0.10, we cannot rely on the built-in method, as it may not be present in older Node.js and browser versions. Hence, manual iteration is our only portable option.

@kgryte kgryte merged commit de50d0a into stdlib-js:develop Jun 18, 2024
@kgryte kgryte deleted the reverse/toReversed branch June 18, 2024 17:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Feature Issue or pull request for adding a new feature.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants