-
Notifications
You must be signed in to change notification settings - Fork 22
Add preferences abstraction #12
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
Conversation
@faern I think your implementation is pretty good. When this PR got merged, i will rebase PR-8. |
@@ -13,8 +13,7 @@ | |||
//! [`SCPreferences`]: https://developer.apple.com/documentation/systemconfiguration/scpreferences |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Review status: 0 of 2 files reviewed at latest revision, 2 unresolved discussions. system-configuration/src/preferences.rs, line 60 at r3 (raw file):
I fixed a bug here now where the reference count was wrong. Since Comments from Reviewable |
Review status: 0 of 2 files reviewed at latest revision, 2 unresolved discussions. system-configuration/src/preferences.rs, line 13 at r2 (raw file): Previously, LuoZijun (寧靜) wrote…
Thanks. Done Comments from Reviewable |
Reviewed 1 of 2 files at r1, 1 of 1 files at r4. system-configuration/src/preferences.rs, line 32 at r4 (raw file):
Am I right that we do not implement any other methods except instantiation because we don't directly access them? Comments from Reviewable |
Review status: all files reviewed at latest revision, 3 unresolved discussions. system-configuration/src/preferences.rs, line 32 at r4 (raw file): Previously, pronebird (Andrei Mihailov) wrote…
Exactly. We don't need anything else for now. We just need access to the Comments from Reviewable |
Review status: all files reviewed at latest revision, 1 unresolved discussion. Comments from Reviewable |
d037442
to
55477ed
Compare
Reviewed 1 of 1 files at r5. Comments from Reviewable |
PR #8 was growing quite a lot. So I felt like splitting out some of the code for separate review in order to shrink that PR a bit.
This PR introduces the safe abstraction for
SCPreferences
. The first commit is the code from @LuoZijun that I cherry picked from the other PR branch. The second commit is where I add some simplifying constructors making it easier to construct the type for the most common use case.@LuoZijun please add your feedback to this as well.
Please note that all constructors take
&CFString
instead of&str
. This is in line with how we have done indynamic_store.rs
and also with how other crates implementing Apple APIs usually do. Example: https://docs.rs/core-graphics/0.13.0/core_graphics/font/struct.CGFont.html#method.from_nameThis change is