Remove unwrap()s from context.rs

bheisler / RustaCUDA

Rusty wrapper for the CUDA Driver API

Apache License 2.0

758 stars 60 forks source link

Remove unwrap()s from context.rs #26

Closed alphastrata closed 5 years ago

alphastrata commented 5 years ago

I removed all the ones I could.

cargo test --doc 97 Passes, 2 Ignored, 0 Failing.

Hope it's acceptable.

bheisler commented 5 years ago

Thanks for the pull request! It might be a while before I can review this properly.

bheisler commented 5 years ago

For the most part this looks good, but there are a bunch of minor things that will need to be cleaned up.

There are a number of what appear to be copy-paste errors and other minor things.
There is also some trailing whitespace in the examples which is causing the Rustfmt check in the CI builds to fail.
In other circumstances, it would make sense to add another level of indentation when wrapping a function around some code, but since the reader of the examples can't see the fn main code, for these examples the indentation shouldn't be changed.

Other comments:

I won't be too picky about it, but in future it would be a good idea to remove commits like your test and test2 commits from the history.

alphastrata commented 5 years ago

Should I just refork your repo & then submit a new pull request, once I've made those cleanups and possibly others?

bheisler commented 5 years ago

No, just push more commits fixing the issues I raised in my review.

You may want to read up on the git rebase command and how it can be used to rewrite the history of a branch, but don't worry about it too much. I can fix it when I do the merge later.

alphastrata commented 5 years ago

Whoops, while messing around with git I accidentally closed this. Sorry.

alphastrata commented 5 years ago

This should be good to go.

If it's acceptable, I'll start working on the other unwrap() calls in device.rs etc ~

rusch95 commented 5 years ago

I split up device.rs, so make sure to pull from master before doing that

On Tue, Dec 25, 2018, 5:34 PM Jeremy Francis <notifications@github.com wrote:

This should be good to go.

If it's acceptable, I'll start working on the other unwrap() calls in device.rs etc ~

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/bheisler/RustaCUDA/pull/26#issuecomment-449879222, or mute the thread https://github.com/notifications/unsubscribe-auth/AKUNKKdS1eAjA8kANlt6Q9MGzVKezsL4ks5u8rYNgaJpZM4ZfT9i .

bheisler commented 5 years ago

Thanks for the pull request!