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

Minor Cleanups for 0.2 #136

Merged
merged 5 commits into from
Apr 27, 2020
Merged

Minor Cleanups for 0.2 #136

merged 5 commits into from
Apr 27, 2020

Conversation

josephlr
Copy link
Member

  • Increase Rust Minimum version to 1.33. This lets us:
    • Remove cfg(getrandom_uwp) and just use target_vendor
    • Turn a macro into a function
  • Use a consistent include style
  • Use asserts consistently
  • Simplify iOS implementation, remove SecRandom type (as it's not part of the iOS API)

See the individual commits for more information

@josephlr josephlr added this to the 0.2 milestone Feb 26, 2020
@josephlr josephlr mentioned this pull request Feb 26, 2020
Closed
9 tasks
dhardy
dhardy approved these changes Feb 28, 2020
View reviewed changes
Copy link
Member

@dhardy dhardy left a comment

Choose a reason for hiding this comment

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

Rust 1.33 is a year old at this point; I don't see much issue in using it. We may need to bump the Rand version number to depend on this, but will need to soon anyway. This should be mentioned in the CHANGELOG however.

@josephlr
error: Make internal_error! a function
87886c5 
Also make Error::code a const fn. These increase the Min Rust version to 1.33
@josephlr
assert: Make use of asserts consistent
6c603fb 
- Use debug asserts in code
- Use normal asserts in tests
- Use *assert_eq! methods when possible
- Remove unnecessary asserts
@josephlr
build: Remove getrandom_uwp cfg
51d0411 
As our minimum rust version is 1.33, we can just use `target_vendor`.
@josephlr
ios: Don't use SecRandom type
6d3b8bc 
The type of the `rnd` parameter is SecRandomRef which is an alias for an
"Opaque Pointer". This is better represented in Rust as a c_void pointer.

We also know that kSecRandomDefault is NULL, so we can simplify the code.

https://developer.apple.com/documentation/security/1399291-secrandomcopybytes
https://developer.apple.com/documentation/security/secrandomref
https://developer.apple.com/documentation/swift/imported_c_and_objective-c_apis/using_imported_c_functions_in_swift
@josephlr
fmt: Use consistent include style
1dcc409
@josephlr josephlr merged commit 74eca5e into rust-random:0.2 Apr 27, 2020
@josephlr josephlr deleted the cleanup branch April 27, 2020 07:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@dhardy dhardy dhardy approved these changes

@newpavlov newpavlov Awaiting requested review from newpavlov

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
0.2
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@josephlr @dhardy