search

maidsafe-archive / safe_dns

Other
8 stars 12 forks source link

MAID-1318 Follow changes in safe_nfs and safe_client #42

Closed sidred closed 9 years ago

sidred commented 9 years ago

Convert all (::routing::NameType, u64) to use the struct from safe_nfs ::safe_nfs::metadata::directory_key::DirectoryKey

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.

sidred commented 9 years ago

Made the recommended changes.

sidred commented 9 years ago

Formatting changes done. 1) Do you have a set of formatting rules anywhere. The only one I found was for C++. 2) Do I use the full path everywhere or define it at the top i.e. use ::safe_nfs::metadata::directory_key::DirectoryKey; at the top of the file and use DirectoryKey everywhere?

ustulation commented 9 years ago

1) sure: https://gist.github.com/ustulation/086a62d4f64d02542afa 2) No

ustulation commented 9 years ago

Ok .. i suppose i can go for a merge on this one .. Can you pls move the task to In Review and assign it to me ... also do not forget to mention your bit-coin address here as an overall comment (like this one of mine) .. nice one @sidred10

sidred commented 9 years ago

My bitcoin address is 17fyuuSaJ1JTmvDtRJUSEhAt6VFEYBQ8EB

This also includes changes for MAID-1314. I am currently not assigned to that task. Not sure if you want to close that too.

ustulation commented 9 years ago

I've assigned it to you - 1314. So if you could move both of them to review (1318 and 1314) and assign it to me, I'll merge this. Also can I discard the PR for 1314 then i suppose as you have included those changes in this one ?

sidred commented 9 years ago

I've close the pr for 1314 and assigned both 1314 and 1318 to you for review.

NickLambert commented 9 years ago

Payment for 1314 and 1318 on the way! Thanks for your help sidred10!