Skip to content

Various fixes and rearrangements #145

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 5 commits into from
Jan 24, 2018

Conversation

faern
Copy link
Contributor

@faern faern commented Jan 21, 2018

A couple of things that I found easy to do that would improve the readability and usability of the crate in various ways. Some might be more subjectively better rather than objectively better. I clogged some unrelated changes into the same PR since the merge process takes ages with Travis being this slow. Anyway, they all fit under the "cleanup" category :) Tell me if you want them split into multiple PRs.

  • Export the sys part of the error module. All other modules do this. I guess it was missed?

  • Fix so the macros are usable from other crates without having to depend on core-foundation-sys or import CFRelease into the scope where they are used.

  • The tests in lib.rs were only testing the dictionary implementations, so I moved them to dictionary.rs

  • I found it a bit undiscoverable that CFRunLoopSource define a method inside of filedescriptor.rs. At the same time it's not ideal to call the ffi-methods of the filedescriptor from runloop.rs. This rearrangement solves both of those problems by exposing CFFileDescriptor::to_run_loop_source that can be used from runloop.rs.


This change is Reviewable

@jdm
Copy link
Member

jdm commented Jan 23, 2018

@bors-servo r+
Thanks for doing this cleanup! These are nice improvements.

@bors-servo
Copy link
Contributor

📌 Commit 2387564 has been approved by jdm

@bors-servo
Copy link
Contributor

⌛ Testing commit 2387564 with merge 0389b23...

bors-servo pushed a commit that referenced this pull request Jan 23, 2018
Various fixes and rearrangements

A couple of things that I found easy to do that would improve the readability and usability of the crate in various ways. Some might be more subjectively better rather than objectively better. I clogged some unrelated changes into the same PR since the merge process takes ages with Travis being this slow. Anyway, they all fit under the "cleanup" category :) Tell me if you want them split into multiple PRs.

* Export the sys part of the error module. All other modules do this. I guess it was missed?

* Fix so the macros are usable from other crates without having to depend on `core-foundation-sys` or import `CFRelease` into the scope where they are used.

* The tests in `lib.rs` were only testing the dictionary implementations, so I moved them to `dictionary.rs`

* I found it a bit undiscoverable that `CFRunLoopSource` define a method inside of `filedescriptor.rs`. At the same time it's not ideal to call the ffi-methods of the filedescriptor from runloop.rs. This rearrangement solves both of those problems by exposing `CFFileDescriptor::to_run_loop_source` that can be used from `runloop.rs`.

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/core-foundation-rs/145)
<!-- Reviewable:end -->
@bors-servo
Copy link
Contributor

☀️ Test successful - status-travis
Approved by: jdm
Pushing 0389b23 to master...

@bors-servo bors-servo merged commit 2387564 into servo:master Jan 24, 2018
@faern faern deleted the various-fixes-and-rearrangements branch January 24, 2018 07:53
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