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

Use murmur3 for hashing #18

Merged
merged 1 commit into from
Dec 9, 2024

Conversation

samfrench
Copy link
Contributor

This PR updates the hash library from "fasthash" to "murmur3" due to compilation issues on a Mac. Looking through the fasthash repository there are many compilation issues across various devices.

This was raised in #17 and a PR was suggested.

@MarkBiesheuvel
Copy link
Owner

Thank you for the pull request!

I don't have automated testing set up, so I had to pull the branch and run the test locally. All looks good.

$ cargo test --all-features --release
   Compiling murmur3 v0.5.2
   Compiling optimizely v0.3.0 (/home/mark/projects/optimizely-rust-sdk/optimizely)
   Compiling results-demo v0.1.0 (/home/mark/projects/optimizely-rust-sdk/examples/results-demo)
   Compiling performance-test v0.1.0 (/home/mark/projects/optimizely-rust-sdk/examples/performance-test)
   Compiling network-test v0.1.0 (/home/mark/projects/optimizely-rust-sdk/examples/network-test)
   Compiling srm-test v0.1.0 (/home/mark/projects/optimizely-rust-sdk/examples/srm-test)
    Finished `release` profile [optimized] target(s) in 2.33s
     Running unittests src/main.rs (target/release/deps/network_test-2bae3fd2e66f0f79)

running 0 tests

test result: ok. 0 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.00s

     Running unittests src/lib.rs (target/release/deps/optimizely-9d1a896ee2bfb8bc)

running 1 test
test datafile::traffic_allocation::tests::variation ... ok

test result: ok. 1 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.00s

     Running tests/client_initialization.rs (target/release/deps/client_initialization-e245101cebd67b79)

running 5 tests
test with_invalid_array_properties ... ok
test with_invalid_json ... ok
test with_missing_properties ... ok
test with_fixed_datafile ... ok
test with_sdk_key ... ok

test result: ok. 5 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.36s

     Running tests/decisions.rs (target/release/deps/decisions-7e68bceb86f468a7)

running 3 tests
test invalid_flag ... ok
test qa_rollout_flag ... ok
test buy_button_flag ... ok

test result: ok. 3 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.00s

     Running tests/user_context.rs (target/release/deps/user_context-0f3b855262d9eae4)

running 3 tests
test user_context_with_attributes ... ok
test user_context_set_attribute ... ok
test user_context_track_event ... ok

test result: ok. 3 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.00s

     Running unittests src/main.rs (target/release/deps/performance_test-2f68ff890a176041)

running 0 tests

test result: ok. 0 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.00s

     Running unittests src/main.rs (target/release/deps/results_demo-35fada1f3af5dfca)

running 0 tests

test result: ok. 0 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.00s

     Running unittests src/main.rs (target/release/deps/srm_test-88139588bd926548)

running 0 tests

test result: ok. 0 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.00s

   Doc-tests optimizely

running 5 tests
test optimizely/src/client.rs - client::Client (line 19) ... ok
test optimizely/src/decision/decide_options.rs - decision::decide_options::DecideOptions (line 5) ... ok
test optimizely/src/client/initialization.rs - client::initialization::UninitializedClient (line 15) ... ok
test optimizely/src/client/user.rs - client::user::UserContext (line 24) ... ok
test optimizely/src/lib.rs - (line 11) ... ok

test result: ok. 5 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.65s

@MarkBiesheuvel
Copy link
Owner

MarkBiesheuvel commented Dec 9, 2024

Haha, I turns out I did have automated testing set up. Completely forgot about that. Merging now!

@MarkBiesheuvel MarkBiesheuvel merged commit e6d84bf into MarkBiesheuvel:main Dec 9, 2024
1 check passed
@samfrench samfrench deleted the update-hash-library branch December 9, 2024 15:07
@samfrench
Copy link
Contributor Author

@MarkBiesheuvel, thank you for the merge. I ran tests before the PR but wasn't sure myself what was setup to run or if there would need to be any changes done to it.

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.

2 participants