-
Notifications
You must be signed in to change notification settings - Fork 25
Implemeted mv and rmdir #43
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
base: main
Are you sure you want to change the base?
Conversation
|
On make and make qemu ran fine. Mvtest performed well, providing clear output to inform the user, and additional commands were provided at the end. Should the user wish to manually test, the "mv -i" command is effective. Rmdir works perfectly. The documentation is excellent, and there are also test programs available for a quick demo. The code formatting is generally consistent and follows the style of the other user-space utilities in the project. There are no major formatting issues to address. The code performs its intended function. Good Job! High Level Checks: Code Checks: |
|
The functionality worked as expected from following the documentation. There weren't any outstanding formating issues within the files, everything seemed cohesive. The documentation was laid out great, with how to build and test the code. I do not think there are any improvements that need to be made! Great job! High Level Checks:
Code Checks:
|
|
Great job with documentation and formatting! I really like how you added automated tests to your repo, that was really helpful as a first time viewer. I was able to use the -f, -i, and -v flags just fine for mv. I also found that rmdir works perfectly as well. I don’t think I have suggestions to improve readability, efficiency, or correctness because I found it all very intuitive. High Level Checks:
|
|
This looks great! Nice work. Here are some things I noticed as I went through this:
I am a bit surprised that the reviewers above weren't able to come up with any more suggestions for you. It's a very high quality project but there's always more we can do (I'm only saying this so we can feel like we have job security). 5/5. |
Summary
We implemented: mv (move/rename files) and rmdir (remove empty directories), along with their respective automated test programs mvtest and rmdirtest.
Features Implemented
mv
rmdir
Build Instructions
To build:
$ make clean
$ make qemu
or:
$ make clean && make qemu
This will compile both commands (
mvandrmdir) and their corresponding test programs (mvtest,rmdirtest).You can verify the binaries are included by running:
$ ls
You should see entries like:
mv
mvtest
rmdir
rmdirtest
Automated Tests
mvtest
Validates:
rmdirtest
Validates:
Documentation
Added Markdown documentation:
Each document includes both manual examples and expected test outputs.
How to Test
After building the user programs, run:
$ mvtest
Expected results:
Test: simple move... OK
Test: -f overwrite... OK
Test: -v verbose output... OK
Test: move into directory... OK
Test: same file detection... mv: 's.txt' and './s.txt' are the same file
OK
Test: move into dir with trailing slash... OK
Test: move file between directories... OK
Manual test: run
mv -i src dstand try typing y/n yourself.$ rmdirtest
Expected results:
Test: remove one empty directory... OK
Test: remove multiple directories... OK
Test: refuse non-empty directory... rmdir: full: directory not empty
OK
Test: refuse non-directory... rmdir: afile.txt: not a directory
OK
Test: -v prints confirmation... OK
Manual: try
rmdirwith multiple args or flag -v yourself in the shell.