Skip to content

Conversation

ahmad-kashkoush
Copy link
Contributor

Resolves #3145 .

Description

What is the purpose of this pull request?

This pull request:

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

Copy link
Contributor

@stdlib-bot stdlib-bot left a comment

Choose a reason for hiding this comment

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

👋 Hi there! 👋

And thank you for opening your first pull request! We will review it shortly. 🏃 💨

@stdlib-bot
Copy link
Contributor

stdlib-bot commented Nov 27, 2024

Coverage Report

Package Statements Branches Functions Lines
array/fixed-endian-factory $\color{red}932/1189$
$\color{green}+78.39\%$
$\color{red}88/97$
$\color{green}+90.72\%$
$\color{red}16/27$
$\color{green}+59.26\%$
$\color{red}932/1189$
$\color{green}+78.39\%$

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

@headlessNode headlessNode added the Feature Issue or pull request for adding a new feature. label Nov 27, 2024
@headlessNode
Copy link
Member

@ahmad-kashkoush the title should start with "feat:"

@ahmad-kashkoush ahmad-kashkoush changed the title add includes() method to array/fixed-endian-factory feat: add includes() method to array/fixed-endian-factory Nov 27, 2024
@ahmad-kashkoush
Copy link
Contributor Author

@ahmad-kashkoush the title should start with "feat:"

Done, may you review PR?

Copy link
Member

@headlessNode headlessNode left a comment

Choose a reason for hiding this comment

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

@ahmad-kashkoush A few suggestions regarding the documentation, otherwise looks good. A more thorough review will be done by the maintainers. Also you should remove the parenthesis after includes in the title.

@ahmad-kashkoush ahmad-kashkoush changed the title feat: add includes() method to array/fixed-endian-factory feat: add includes method to array/fixed-endian-factory Nov 28, 2024
ahmad-kashkoush and others added 3 commits November 28, 2024 12:46
Co-authored-by: Muhammad Haris <[email protected]>
Signed-off-by: Ahmed_Kashkoush <[email protected]>
Co-authored-by: Muhammad Haris <[email protected]>
Signed-off-by: Ahmed_Kashkoush <[email protected]>
Co-authored-by: Muhammad Haris <[email protected]>
Signed-off-by: Ahmed_Kashkoush <[email protected]>
@ahmad-kashkoush
Copy link
Contributor Author

Good evening, @kgryte may you this a review

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, @ahmad-kashkoush!

@kgryte
Copy link
Member

kgryte commented Dec 3, 2024

@ahmad-kashkoush Would you mind resolving the merge conflicts? Once those are resolved, we can merge. Also, so that methods are listed in alphabetical order, includes should come before indexOf in both the README and main.js file.

@kgryte kgryte added the Needs Changes Pull request which needs changes before being merged. label Dec 3, 2024
changed its place before indexOf so file ordered alphatically
@Planeshifter Planeshifter added the Ready To Merge A pull request which is ready to be merged. label Dec 4, 2024
@stdlib-bot
Copy link
Contributor

PR Commit Message

feat: add `includes` method to `array/fixed-endian-factory`

PR-URL: https://github.com/stdlib-js/stdlib/pull/3283
Closes: https://github.com/stdlib-js/stdlib/issues/3145

Co-authored-by: Ahmed_Kashkoush <[email protected]>
Co-authored-by: Athan Reines <[email protected]>
Co-authored-by: Muhammad Haris <[email protected]>
Co-authored-by: ahmad-kashkoush <[email protected]>
Reviewed-by: Athan Reines <[email protected]>
Signed-off-by: Ahmed_Kashkoush <[email protected]>
Signed-off-by: Athan <[email protected]>

Please review the above commit message and make any necessary adjustments.

@Planeshifter Planeshifter merged commit 1d10ce5 into stdlib-js:develop Dec 4, 2024
18 checks passed
@kgryte kgryte removed the Needs Changes Pull request which needs changes before being merged. label Dec 4, 2024
jangnathan pushed a commit to jangnathan/stdlib that referenced this pull request Dec 7, 2024
PR-URL: stdlib-js#3283
Closes: stdlib-js#3145

Co-authored-by: Athan Reines <[email protected]>
Co-authored-by: Muhammad Haris <[email protected]>
Reviewed-by: Athan Reines <[email protected]>
Reviewed-by: Muhammad Haris <[email protected]>
Signed-off-by: Ahmed_Kashkoush <[email protected]>
Signed-off-by: Athan Reines <[email protected]>
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. Ready To Merge A pull request which is ready to be merged.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[RFC]: add includes method to array/fixed-endian-factory

5 participants