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

CpuId reader trait and serialize feature fix #140

Merged
merged 17 commits into from Apr 17, 2023
Merged

Conversation

gz
Copy link
Owner

@gz gz commented Mar 1, 2023

@codecov
Copy link

codecov bot commented Mar 1, 2023

Codecov Report

Merging #140 (2d13588) into master (a837dba) will increase coverage by 2.12%.
The diff coverage is 36.36%.

❗ Current head 2d13588 differs from pull request most recent head aab103c. Consider uploading reports for the commit aab103c to get more accurate results

@@            Coverage Diff             @@
##           master     #140      +/-   ##
==========================================
+ Coverage   39.34%   41.46%   +2.12%     
==========================================
  Files           4        4              
  Lines        4072     3849     -223     
==========================================
- Hits         1602     1596       -6     
+ Misses       2470     2253     -217     
Impacted Files Coverage Δ
src/bin/cpuid.rs 5.55% <0.00%> (+1.20%) ⬆️
src/display.rs 0.00% <0.00%> (ø)
src/extended.rs 85.71% <ø> (+2.35%) ⬆️
src/lib.rs 52.32% <52.83%> (+1.06%) ⬆️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

@mkeeter
Copy link
Contributor

mkeeter commented Mar 2, 2023

This looks good to me!

@gz
Copy link
Owner Author

gz commented Mar 2, 2023

Thanks for taking a look! I think the remaining thing I'm not sure about yet is if the default argument with NativeCpuIdReader is a good idea (if done solely for x86). Maybe I'll write some code examples later on the mac and see how weird it gets.

@gz
Copy link
Owner Author

gz commented Apr 17, 2023

Ok so better late than never... I'm going to merge this now and release v11.

I did remove the default generic argument again, because even without it I don't think it's too bad for migration for users to v11.

@gz gz merged commit 898d010 into master Apr 17, 2023
5 checks passed
@mkeeter
Copy link
Contributor

mkeeter commented Apr 17, 2023

Thanks for merging this in! Can you please push a release to crates.io as well, so we can update our dependencies?

@gz
Copy link
Owner Author

gz commented Apr 17, 2023

Just released v11

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.

Serialization issue
2 participants