miniserve
miniserve copied to clipboard
svenstaro
File & directory deletion feature
Closes #397.
Todo
- [x] CLI options
- [x] Backend API
- [x] Frontend controls
- [x] Tests
https://github.com/svenstaro/miniserve/blob/82dd82615ade9265630a178165c66002b0b9314c/src/main.rs#L329-L330
I am curious why the upload feature is implemented like so. I guess it's good for unforeseen future extensibility, but there is semantic duplication between the /upload endpoint and the POST method.
The real question I want to ask, is should I implement the deletion feature in a similar manner? I.e. something like this:
app.service(web::resource("/delete").route(web::delete().to(file_rm::rm_file)));
Are you suggesting it might have been better to just have POST always try to upload stuff? It would be simpler for sure but as you are saying, it's not great if we ever want a second POST for something. I think this is currently an ok compromise.
I see how this is a bit of a conundrum with delete. I also want to add WebDAV at some point and that will introduce even more semantics. Perhaps it's the best to go with what you suggested just now and eat the semantic duplication for the time being. It's not like it can't be changed later.
Hi,
unfortunately I can't directly help... just wanted to say that I appreciate the effort and if it gets done at some point, my life would be somehow better :)
Hi,
unfortunately I can't directly help... just wanted to say that I appreciate the effort and if it gets done at some point, my life would be somehow better :)
Thanks for reminding me. I have too many things on hand and honestly just forgot about this whole matter. I will find some time to work on it this week, hopefully.
Could you add some tests? Since this is going to be deleting things, it needs some proper tests.
Could you add some tests? Since this is going to be deleting things, it needs some proper tests.
Yeah I'm working on that on and off. The tests are actually written but several are failing. My original intention was to combine the tests and the fixes into one commit, but I guess there's no harm in splitting them up. I'll push the tests first and then look to fix them.
If you can spare some time, please take a look at the test suite; make sure they are testing the correct behaviour. Thanks.
Yeah I'm working on that on and off. The tests are actually written but several are failing. My original intention was to combine the tests and the fixes into one commit, but I guess there's no harm in splitting them up. I'll push the tests first and then look to fix them.
No worries. There's no rush to get this in. Just take your time and make sure things look good.
If you can spare some time, please take a look at the test suite; make sure they are testing the correct behaviour. Thanks.
I will try to squeeze a review in but no promises, I'm fairly low on time currently.