search

maidsafe-archive / safe_dns

Other
8 stars 12 forks source link

MAID-1314 done #39

Closed sidred closed 9 years ago

sidred commented 9 years ago

Only one unwrap found on getting the lock for client. Converted it to use eval_result!() using the convention from other crates in the project

maidsafe-highfive commented 9 years ago

Thanks for the pull request, and welcome! The MaidSafe team is excited to review your changes, and you should hear from @ustulation (or someone else) soon.

If any changes to this PR are deemed necessary, please add them as extra commits. This ensures that the reviewer can see what has changed since they last reviewed the code. The way GitHub handles out-of-date commits, this should also make it reasonably obvious what issues have or haven't been addressed. Large or tricky changes may require several passes of review and changes.

Please see CONTRIBUTOR.md for more information.

ustulation commented 9 years ago

There are many more places this needs to be addressed. Please look carefully. Also you will need to start this JIRA task and assign it to yourself

sidred commented 9 years ago

This is the only one in the code. The rest all are in the tests. Do you want those converted as well?

ustulation commented 9 years ago

yes pls

sidred commented 9 years ago

Now checking ok(try!()) blocks

sidred commented 9 years ago

Done.

sidred commented 9 years ago

Not sure why the travis tests are failing. The example and the tests run without errors on my mcahine.

dirvine commented 9 years ago

From the error it would appear you are using a different version of a dependency than travis (which uses latest). Did you do a cargo update then cargo test or perhaps using a local copy of dependencies?

`examples/simulate_browser.rs:148 let mut writer = try!(file_helper.create(HOME_PAGE_FILE_NAME.to_string(), vec![], dir_listing)); ^~~

:1:1: 6:48 note: in expansion of try! examples/simulate_browser.rs:148:22: 148:100 note: expansion site examples/simulate_browser.rs:148:87: 148:98 help: run`rustc --explain E0308`to see a detailed explanation examples/simulate_browser.rs:159:46: 159:55 error: no method named`get_key`found for type`(safe_nfs::directory_listing::DirectoryListing, core::option::Option)`in the current scope examples/simulate_browser.rs:159 let dir_key = updated_parent_dir_listing.get_key(); ^~~~~~~~~ examples/simulate_browser.rs:164:71: 164:80 error: the type of this value must be known in this context examples/simulate_browser.rs:164 (service_name, (dir_key.0.clone(), dir_key.1)), `
sidred commented 9 years ago

Ok, so travis is using the latest commit from master branch for safe_nfs while cargo is using tagged version 0.10. Looks like there have been some breaking changes in safe_nfs and the example needs to be updated. I will hold off until pending pull requests in safe_nfs are done.

sidred commented 9 years ago

Closing as all changes in this request are in the pull request for MAID-1318