-
Notifications
You must be signed in to change notification settings - Fork 49
Adding array methods and graphic art #78
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
|
I think your tests are great and your implementation is straight forward and readable. You definitely achieved what you outlined in the issue! Some thoughts/advice:
High Level Checks:
Code Checks:
|
|
I can code review this |
|
I would like to code review this |
|
High Level Checks:
Overall a great project. Only suggestions are making the graphic art work, and adding other programs that allow for custom input so that I can test it more easily. With small improvements I think your project will be amazing!! |
|
All the features mentioned in the issue are implemented! I think your project is great and checks all the requirements mentioned.
High level checks:
Code Checks:
|
|
Had to modify the graphic art files to remove std libraries and include the xv6 headers, then add to Makefile, but after that they work great. You should fix those small issues so others can run this directly. Array tests look good. I'd probably add more exhaustive tests with additional arrays here, but it's a good start. You could run the entire procedure in main but with several different arrays in a loop and then provide the expected outputs. With these fixes and those above you can earn +0.5 to your project score. |
Some basic array method are added, and some graphic art is added to user space
Modification:
Makefile - arraytests.c is added to show tests of new methods
user - array.h, array.c, vagabond.c, mittens.c
Limitations:
Sort only works on integer arrays, identical with getMin and getMax method
graphic art files vagabound.c and mittens.c don't run in kernel, although a quick change allows that.