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 math/base/special/lcmf #3098

Merged
merged 11 commits into from
Dec 2, 2024
Merged

feat: add math/base/special/lcmf #3098

merged 11 commits into from
Dec 2, 2024

Conversation

aayush0325
Copy link
Contributor

@aayush0325 aayush0325 commented Nov 11, 2024 •
edited by gunjjoshi
Loading

Resolves a part of #649.

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.

Open to reviews!!

Checklist

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

@stdlib-js/reviewers

@stdlib-bot stdlib-bot added the Math Issue or pull request specific to math functionality. label Nov 11, 2024
@aayush0325
Copy link
Contributor Author

ready for review @Planeshifter !

@kgryte kgryte added Feature Issue or pull request for adding a new feature. Needs Review A pull request which needs code review. labels Nov 18, 2024
@kgryte kgryte requested a review from gunjjoshi November 18, 2024 04:13
@kgryte
Copy link
Member

kgryte commented Nov 18, 2024

/stdlib merge

@kgryte
Copy link
Member

kgryte commented Nov 18, 2024

/stdlib update-copyright-years

@gunjjoshi
Copy link
Member

We also have a PR for the same package at #3096.

@kgryte
Copy link
Member

kgryte commented Nov 18, 2024

@gunjjoshi Hmm...indeed. That other PR seems like it still needs a bit of work. Do you think this one is in better shape?

@gunjjoshi
Copy link
Member

@kgryte Yes, I think this one would need only a little changes, as compared to a lot more corrections, still to be pointed out in the other PR.

@aayush0325
Copy link
Contributor Author

aayush0325 commented Nov 18, 2024 •
edited
Loading

We also have a PR for the same package at #3096.

Oh sorry my bad. I had been working on this since my PR for math/base/special/gcdf #2997. If the other PR is ready to merge then i can close this, my apologies.

@aayush0325
Copy link
Contributor Author

How should we proceed here then @kgryte ?

gunjjoshi
gunjjoshi requested changes Nov 19, 2024
View reviewed changes
lib/node_modules/@stdlib/math/base/special/lcmf/README.md Outdated Show resolved Hide resolved
lib/node_modules/@stdlib/math/base/special/lcmf/benchmark/benchmark.js Show resolved Hide resolved
lib/node_modules/@stdlib/math/base/special/lcmf/benchmark/benchmark.native.js Show resolved Hide resolved
lib/node_modules/@stdlib/math/base/special/lcmf/benchmark/c/native/benchmark.c Outdated Show resolved Hide resolved
lib/node_modules/@stdlib/math/base/special/lcmf/benchmark/c/native/benchmark.c Outdated Show resolved Hide resolved
lib/node_modules/@stdlib/math/base/special/lcmf/lib/main.js Outdated Show resolved Hide resolved
lib/node_modules/@stdlib/math/base/special/lcmf/lib/main.js Outdated Show resolved Hide resolved
lib/node_modules/@stdlib/math/base/special/lcmf/src/main.c Outdated Show resolved Hide resolved
lib/node_modules/@stdlib/math/base/special/lcmf/src/main.c Outdated Show resolved Hide resolved
lib/node_modules/@stdlib/math/base/special/lcmf/test/test.js Outdated Show resolved Hide resolved
@gunjjoshi
Copy link
Member

This PR should be good to go once the comments have been addressed.

@gunjjoshi gunjjoshi removed the Needs Review A pull request which needs code review. label Nov 19, 2024
@gunjjoshi gunjjoshi added the Needs Changes Pull request which needs changes before being merged. label Nov 19, 2024
@aayush0325
Copy link
Contributor Author

aayush0325 commented Nov 19, 2024 •
edited
Loading

@gunjjoshi resolved the issues with the commit history of this branch now this only contains the required files, with the requested changes.

@aayush0325 aayush0325 requested a review from gunjjoshi November 19, 2024 16:18
@gunjjoshi gunjjoshi removed the Needs Changes Pull request which needs changes before being merged. label Nov 19, 2024
@kgryte
Copy link
Member

kgryte commented Nov 22, 2024

/stdlib merge

@stdlib-bot stdlib-bot added the bot: In Progress Pull request is currently awaiting automation. label Nov 22, 2024
@kgryte kgryte added Needs Review A pull request which needs code review. and removed bot: In Progress Pull request is currently awaiting automation. labels Nov 22, 2024
@stdlib-bot
Copy link
Contributor

Coverage Report

Package Statements Branches Functions Lines
math/base/special/lcmf $\color{green}166/166$
$\color{green}+100.00\%$ 		$\color{green}14/14$
$\color{green}+100.00\%$ 		$\color{green}2/2$
$\color{green}+100.00\%$ 		$\color{green}166/166$
$\color{green}+100.00\%$

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

@aayush0325
Copy link
Contributor Author

does this need any changes @gunjjoshi ?

Planeshifter
Planeshifter approved these changes Dec 1, 2024
View reviewed changes
Copy link
Member

@Planeshifter Planeshifter left a comment

Choose a reason for hiding this comment

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

PR looks good; thank you @aayush0325 for your contribution and to @gunjjoshi for reviewing.

@Planeshifter Planeshifter added the Ready To Merge A pull request which is ready to be merged. label Dec 1, 2024
@Planeshifter Planeshifter merged commit c0d083d into stdlib-js:develop Dec 2, 2024
17 of 18 checks passed
nate10j pushed a commit to nate10j/stdlib that referenced this pull request Dec 7, 2024
@aayush0325 @stdlib-bot @nate10j
feat: add math/base/special/lcmf
08226ec 
PR-URL: stdlib-js#3098
Ref: stdlib-js#649

---------

Reviewed-by: Gunj Joshi <gunjjoshi8372@gmail.com>
Reviewed-by: Philipp Burckhardt <pburckhardt@outlook.com> 
Co-authored-by: stdlib-bot <82920195+stdlib-bot@users.noreply.github.com>
@gunjjoshi gunjjoshi mentioned this pull request Dec 17, 2024
Open
1 task
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Planeshifter Planeshifter Planeshifter approved these changes

@gunjjoshi gunjjoshi Awaiting requested review from gunjjoshi

Assignees
No one assigned
Labels
Feature Issue or pull request for adding a new feature. Math Issue or pull request specific to math functionality. Needs Review A pull request which needs code review. Ready To Merge A pull request which is ready to be merged.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@aayush0325 @kgryte @gunjjoshi @stdlib-bot @Planeshifter