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.

Sign up for GitHub

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

Jump to bottom

feat: add toReversed method to array/fixed-endian-factory #3299

Open
wants to merge 19 commits into
base: develop
Choose a base branch
from
Open

feat: add toReversed method to array/fixed-endian-factory #3299

wants to merge 19 commits into from

Conversation

nate10j
Copy link

@nate10j nate10j commented Nov 30, 2024

Resolves #3159

Description

What is the purpose of this pull request?

This pull request:

  • Adds toReversed method to main.js
  • Adds tests for toReversed method in new file test.to_reversed.js
  • Adds two benchmark files for toReversed method, one measuring baseline performance and the other measuring performance as array length increases
  • Added documentation and updated README.md file for toReversed method

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

@stdlib-bot
Copy link
Contributor

stdlib-bot commented Nov 30, 2024 •
edited
Loading

Coverage Report

Package Statements Branches Functions Lines
array/fixed-endian-factory $\color{red}1206/1457$
$\color{green}+82.77\%$ 		$\color{red}160/172$
$\color{green}+93.02\%$ 		$\color{red}24/34$
$\color{green}+70.59\%$ 		$\color{red}1206/1457$
$\color{green}+82.77\%$

The above coverage report was generated for the changes in this PR.

ahmad-kashkoush
ahmad-kashkoush reviewed Nov 30, 2024
View reviewed changes
lib/node_modules/@stdlib/array/fixed-endian-factory/test/test.to_reversed.js

t.strictEqual( instanceOf( out, ctor ), true, 'returns expected value' );
t.notEqual( out, arr, 'returns a new instance' );
t.strictEqual( out.length, expected.length, 'returns expected value' );
Copy link
Contributor

Choose a reason for hiding this comment

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

this test doesn't compare values of expected against values of out, it just compares the length.

Copy link
Author

Choose a reason for hiding this comment

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

made changes

@Planeshifter Planeshifter changed the title feat: add toReversed method to array/fixed-endian-factory feat: add toReversed method to array/fixed-endian-factory Dec 1, 2024
@Planeshifter Planeshifter added the Needs Review A pull request which needs code review. label Dec 1, 2024
@nate10j nate10j force-pushed the feature/fixed-endian-factory-toReverse branch 2 times, most recently from e3001fa to b5ccb4d Compare December 2, 2024 09:09
kgryte
kgryte requested changes Dec 3, 2024
View reviewed changes
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 for working on this @nate10j. Left an initial round of comments.

@kgryte kgryte added Feature Issue or pull request for adding a new feature. Needs Changes Pull request which needs changes before being merged. and removed Needs Review A pull request which needs code review. labels Dec 3, 2024
@nate10j nate10j force-pushed the feature/fixed-endian-factory-toReverse branch 2 times, most recently from 8c1cba3 to 4f6ae97 Compare December 7, 2024 03:45
nate10j and others added 13 commits December 7, 2024 12:04
@nate10j @kgryte
fix: capitalise fail message in array/fixed-endian-factory/benchmark
361340c 
Co-authored-by: Athan <[email protected]>
Signed-off-by: Nate <[email protected]>
@nate10j @kgryte
refactor: edit arr variable declaration in array/fixed-endian-factory…
a43f648 
…/benchmark

Co-authored-by: Athan <[email protected]>
Signed-off-by: Nate <[email protected]>
@nate10j @kgryte
fix: edit fail message in array/fixed-endian-factory/benchmark
4d1c5ce 
Co-authored-by: Athan <[email protected]>
Signed-off-by: Nate <[email protected]>
@nate10j @kgryte
docs: update JSDoc comment toReversed method in array/fixed-endian-fa…
f770a9c 
…ctory

Co-authored-by: Athan <[email protected]>
Signed-off-by: Nate <[email protected]>
@nate10j @kgryte
docs: edit JSDoc comment to toReversed method in array/fixed-endian-f…
6d03899 
…actory

Co-authored-by: Athan <[email protected]>
Signed-off-by: Nate <[email protected]>
@nate10j @kgryte
docs: edit JSDoc comment to toReversed method in array/fixed-endian-f…
c701482 
…actory

Co-authored-by: Athan <[email protected]>
Signed-off-by: Nate <[email protected]>
@nate10j @kgryte
perf: minimize arithmetic in array/fixed-endian-factory
fe317dd 
Co-authored-by: Athan <[email protected]>
Signed-off-by: Nate <[email protected]>
@nate10j
style: add missing space to array/fixed-endian-factory/test
661d0bc 
Co-authored-by: Athan <[email protected]>
Signed-off-by: Nate <[email protected]>

style: add missing space to array/fixed-endian-factory/test

style: add missing space to test for toReversed method
@nate10j
style: remove blank line for toReversed method in array/fixed-endian-…
7f4f8be 
…factory

Co-authored-by: Athan <[email protected]>
Signed-off-by: Nate <[email protected]>

style: remove blank line for toReversed method in array/fixed-endian-factory

style: remove blank line for toReversed method in array/fixed-endian-factory

style: remove blank line in toReversed method
@nate10j
style: remove blank line for toReversed method in array/fixed-endian-…
b595cc2 
…factory

Co-authored-by: Athan <[email protected]>
Signed-off-by: Nate <[email protected]>

style: remove blank line for toReversed method in array/fixed-endian-factory

style: remove blank line in toReversed method in array/fixed-endian-factory

style: remove blank line for toReversed method
@nate10j
docs: remove duplicate JSDoc comment in array/fixed-endian-factory
da97081
@nate10j
test: use hasSameValues method to test in array/fixed-endian-factory
328ba9d
@nate10j
style: insert toReversed in alphabetic order in array/fixed-endian-fa…
0e0154c 
…ctory
@nate10j nate10j force-pushed the feature/fixed-endian-factory-toReverse branch from 4f6ae97 to 0e0154c Compare December 7, 2024 04:16
@nate10j nate10j requested a review from kgryte December 9, 2024 08:36
@stdlib-bot stdlib-bot added the Needs Review A pull request which needs code review. label Dec 9, 2024
@nate10j
Copy link
Author

nate10j commented Dec 18, 2024

@kgryte Thanks for the review, I have made required changes

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ahmad-kashkoush ahmad-kashkoush ahmad-kashkoush left review comments

@kgryte kgryte Awaiting requested review from kgryte

Requested changes must be addressed to merge this pull request.

Assignees
No one assigned
Labels
Feature Issue or pull request for adding a new feature. Needs Changes Pull request which needs changes before being merged. Needs Review A pull request which needs code review.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[RFC]: add toReversed method to array/fixed-endian-factory
5 participants
@nate10j @stdlib-bot @kgryte @ahmad-kashkoush @Planeshifter