8000 hongbo/more random tests by bobzhang · Pull Request #2328 · moonbitlang/core · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

hongbo/more random tests #2328

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

Merged
merged 3 commits into from
Jun 25, 2025
Merged

hongbo/more random tests #2328

merged 3 commits into from
Jun 25, 2025

Conversation

bobzhang
Copy link
Contributor
  • tweak README
  • tweak random interface

Copy link
peter-jerry-ye-code-review bot commented Jun 24, 2025
Duplicated documentation comment for chacha8 function

Category
Maintainability
Code Snippet
///|
/// Create a new random number generator with [seed].
/// @Alert unsafe "Panic if seed is not 32 bytes long"
///|
/// Create a new random number generator with [seed].
/// @Alert unsafe "Panic if seed is not 32 bytes long"
Recommendation
Remove the duplicated documentation block and keep only one instance
Reasoning
Duplicated documentation makes maintenance harder and can lead to inconsistencies when one copy is updated but not the other

The fnalias declaration appears incomplete

Category
Correctness
Code Snippet
///|
pub fnalias Rand::chacha8
Recommendation
Either remove the incomplete fnalias or complete it with proper alias definition
Reasoning
The current fnalias declaration seems to be a dangling reference without a target, which could cause confusion or compilation issues

Optional parameter syntax changed from default value to Option type

Category
Maintainability
Code Snippet
pub fn Rand::new(generator~ : &Source = chacha8()) -> Rand
// changed to
pub fn Rand::new(generator? : &Source) -> Rand
Recommendation
Consider keeping consistent parameter style across the codebase. If changing to Option type, update all related functions to follow the same pattern
Reasoning
Mixed styles of handling optional parameters can make the API less predictable and harder to use correctly. The change from default value to Option type is a significant API change that should be consistent across the module

@coveralls
Copy link
Collaborator

Pull Request Test Coverage Report for Build 7479

Details

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage remained the same at 89.343%

Totals Coverage Status
Change from base Build 7474: 0.0%
Covered Lines: 3521
Relevant Lines: 3941

💛 - Coveralls

@coveralls
Copy link
Collaborator
coveralls commented Jun 24, 2025

Pull Request Test Coverage Report for Build 30

Details

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage remained the same at 89.332%

Totals Coverage Status
Change from base Build 29: 0.0%
Covered Lines: 3517
Relevant Lines: 3937

💛 - Coveralls

@bobzhang bobzhang force-pushed the hongbo/more_random_tests branch from 9d6d05d to e099c6b Compare June 25, 2025 13:42
@bobzhang bobzhang enabled auto-merge (rebase) June 25, 2025 13:42
@bobzhang bobzhang merged commit 3396304 into main Jun 25, 2025
10 checks passed
@bobzhang bobzhang deleted the hongbo/more_random_tests branch June 25, 2025 13:46
@peter-jerry-ye
Copy link
Collaborator

This is a breaking change and broke downstream users.

peter-jerry-ye added a commit that referenced this pull request Jul 2, 2025
peter-jerry-ye added a commit that referenced this pull request Jul 2, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants
0