-
Notifications
You must be signed in to change notification settings - Fork 96
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
Rename and delete datasets #226
Conversation
@@ -50,3 +51,8 @@ pub fn _last_null_pointer_err(method_name: &'static str) -> GdalError { | |||
msg: last_err_msg, | |||
} | |||
} | |||
|
|||
pub fn _path_to_c_string(path: &Path) -> Result<CString> { | |||
let path_str = path.to_string_lossy(); |
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.
😢
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.
I don't know, I just copied the existing behavior. Is there a better way?
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.
On Unix, yes, on Windows it's complicated.
Looks like there's a conflict, can you rebase? |
Looks good, I'll merge it tomorrow unless someone else has remarks. |
* for some definition of "tomorrow" bors r+ |
Build succeeded: |
CHANGES.md
if knowledge of this change could be valuable to users.The
Driver
was missing the methodsrename
anddelete
(and more). I've added them together with a test.I was quite unsure to use
&str
or&Path
as input since we use both in our codebase, in particular,&Path
forDataset::open
. This is why I stuck with&Path
here. For creating theCString
out of the&Path
, I've added a method in util to have a common way to do this.