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.

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

chore: address commit comments and use discrete uniform PRNG #5313

Merged
merged 10 commits into from
Feb 22, 2025

Conversation

rishavtarway
Copy link
Contributor

Resolves #5303.

Description

What is the purpose of this pull request?

This pull request: address the commit comments of (commit 9b455b1

  • changed the value in Readme.md from 10 to 10.0 and in make.c file
  • Also refactored the operation using discreteUniform

Related Issues

Does this pull request have any related issues?

This pull request:

Questions

Any questions for reviewers of this pull request?

Hi, reviewers! Could you please check if I have followed the contributing guidelines properly? If there are any mistakes or areas for improvement in my code or approach, I would appreciate your feedback. I'm open to suggestions for making it better!

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 stdlib-bot added Statistics Issue or pull request related to statistical functionality. Needs Review A pull request which needs code review. labels Feb 19, 2025
@stdlib-bot
Copy link
Contributor

stdlib-bot commented Feb 19, 2025

Coverage Report

Package Statements Branches Functions Lines
stats/base/dists/binomial/median $\color{green}178/178$
$\color{green}+100.00\%$
$\color{green}12/12$
$\color{green}+100.00\%$
$\color{green}2/2$
$\color{green}+100.00\%$
$\color{green}178/178$
$\color{green}+100.00\%$

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

@kgryte kgryte changed the title Refactor : operation using discreteUniform refactor: operation using discreteUniform Feb 19, 2025
@kgryte
Copy link
Member

kgryte commented Feb 19, 2025

@rishavtarway Your PR titles still are not following our conventions.

@rishavtarway
Copy link
Contributor Author

@kgryte sir i have followed the guidelines using refactor changes i have made to this commit
as it include multiple changes from style to refactoring so i used the priority order for refactoring as the PR title

@rishavtarway
Copy link
Contributor Author

@kgryte can you review it once now ?

Copy link
Contributor

@anandkaranubc anandkaranubc left a comment

Choose a reason for hiding this comment

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

Just a few minor changes. Thanks for fixing these areas!

@anandkaranubc anandkaranubc added Needs Changes Pull request which needs changes before being merged. and removed Needs Review A pull request which needs code review. labels Feb 19, 2025
rishavtarway and others added 4 commits February 19, 2025 21:24
…hmark/benchmark.native.js

Co-authored-by: Karan Anand <[email protected]>
Signed-off-by: Rishav Tarway <[email protected]>
…hmark/benchmark.native.js

Co-authored-by: Karan Anand <[email protected]>
Signed-off-by: Rishav Tarway <[email protected]>
…hmark/benchmark.native.js

Co-authored-by: Karan Anand <[email protected]>
Signed-off-by: Rishav Tarway <[email protected]>
…hmark/benchmark.native.js

Co-authored-by: Karan Anand <[email protected]>
Signed-off-by: Rishav Tarway <[email protected]>
@rishavtarway
Copy link
Contributor Author

@anandkaranubc hey can you review the suggested changes now ?

@stdlib-bot stdlib-bot added the Needs Review A pull request which needs code review. label Feb 19, 2025
Signed-off-by: Rishav Tarway <[email protected]>
Signed-off-by: Rishav Tarway <[email protected]>
Signed-off-by: Rishav Tarway <[email protected]>
@anandkaranubc anandkaranubc removed the Needs Review A pull request which needs code review. label Feb 19, 2025
…hmark/benchmark.native.js

Co-authored-by: Karan Anand <[email protected]>
Signed-off-by: Rishav Tarway <[email protected]>
@rishavtarway
Copy link
Contributor Author

@anandkaranubc i have updated the changes suggested

@stdlib-bot stdlib-bot added the Needs Review A pull request which needs code review. label Feb 19, 2025
@rishavtarway
Copy link
Contributor Author

@Planeshifter can you review this PR once ?

Copy link
Contributor

@anandkaranubc anandkaranubc left a comment

Choose a reason for hiding this comment

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

Everything looks good :)

@anandkaranubc anandkaranubc removed Needs Review A pull request which needs code review. Needs Changes Pull request which needs changes before being merged. labels Feb 22, 2025
@Planeshifter Planeshifter changed the title refactor: operation using discreteUniform chore: address commit comments and use discrete uniform PRNG Feb 22, 2025
@Planeshifter Planeshifter added the Ready To Merge A pull request which is ready to be merged. label Feb 22, 2025
@stdlib-bot
Copy link
Contributor

PR Commit Message

chore:  address commit comments and use discrete uniform PRNG

PR-URL: https://github.com/stdlib-js/stdlib/pull/5313
Closes: https://github.com/stdlib-js/stdlib/issues/5303

Co-authored-by: Karan Anand <[email protected]>
Co-authored-by: Rishav Tarway <[email protected]>
Reviewed-by: Philipp Burckhardt <[email protected]>
Reviewed-by: anandkaranubc <[email protected]>
Signed-off-by: Rishav Tarway <[email protected]>

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

@Planeshifter Planeshifter merged commit 6670680 into stdlib-js:develop Feb 22, 2025
45 checks passed
@rishavtarway rishavtarway deleted the fix/#5303 branch February 22, 2025 17:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Ready To Merge A pull request which is ready to be merged. Statistics Issue or pull request related to statistical functionality.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Address commit comments (commit 9b455b1)
5 participants