Conversation
9ff81db to
cffe8e4
Compare
erlingrj
left a comment
There was a problem hiding this comment.
I think this looks good, some questions only!
src/main.rs
Outdated
|
|
||
| fn do_remove_folder(path: &str) -> Result<(), RemoveFolderError> { | ||
| fs::remove_dir_all(path) | ||
| .map_err(|_| RemoveFolderError(format!("Could not delete folder: {}", path))) |
There was a problem hiding this comment.
Does this yield an error if the build folder is not present? Should probably not do that?
There was a problem hiding this comment.
that is indeed a bug, fixed ist last commit
| CommandSpec::Clean => { | ||
| let output_root = &config.apps[0].output_root; | ||
| if let Err(e) = remove_dir_all(&output_root.display().to_string()) { | ||
| error!("lingo was unable to delete build folder! {e}"); |
There was a problem hiding this comment.
More or less the same error message is produced by remove_dir_all
There was a problem hiding this comment.
yes remove_dir_all produces and error and here we handle it.
There was a problem hiding this comment.
But it seems like the error is: "lingo was unable to delete build folder! Could not delete folder: ..." which seems a little redundant
| } | ||
| } | ||
| } | ||
| CommandSpec::Clean => { |
There was a problem hiding this comment.
Should we also remove Lingo.toml here?
edwardalee
left a comment
There was a problem hiding this comment.
In my Makefile, make clean also removes Lingo.lock. If I don’t remove this file, then lingo build isn’t much use because it just checks out same previous version of the library repo that I’m trying to update. Is there a separate mechanism in Lingo for updating a library?
What if I want to update one library and not another? Removing Lingo.lock won’t do that.
Also, there are warning that should be dealt with.
We should probably stay inspired by
Which warning? Is it #60 ? |
|
The purpose of the file If you remove the |
No description provided.