Cloning and running the code took some time. I had to figure a couple of things since the environment setup didn't have any details except the commands. The READ ME file was helpful either way. It was useful since it had some instruction and information about a couple of files and what their tasks.
The code followed the general guidelines and style. The good thing about this project that the majority of the files were short so the code could be easily be represented and styled. It just makes everything make sense. There are multiple READ ME files. I found each directly has one which I believe it is helpful with a complicated project like this where you need to understand what's going on before you compile it. Otherwise, it would be hard.
The code implemented really good in the way that it was written. Looking to some files as scraper.py and generate.py, there are very nice and organized well. I think it was helpful that there was not a lot of code involved in this project.
The unit test is built into the code which is good. It gives me a good idea as a tester or user about what the project should perform. Sometimes I feel like I wish if one of the members of the group walks me through the testing to understand some of these numbers. In general, they are there and could be understood better with some maintenance.
I wish if the requirement file had more information and more details about the project. I find it to be way short for me to grip the project idea and where their final destination ends. More information about the group teams would be good. Good job, on a cool project!
Build: It seems like there might be some backend restrictions on running the program, but the documentation on the readme was clear and straightforward. The team provided step by step guides and specified the division on the project files to reduce confusion.
Legibility: Yes, the code is simple to understand and follows conventions developing good software. The team has also made their code variable easy to understand and straightforward. Looking through their code files as well, variables follow naming conventions and are consistent.
Report: The team also named the different project files with easy to understand conventions and provided a readme file for both parts of the project.
Implementation: The code is much more efficient than what it used to be, the team has managed to move everything to python making things much faster and much easier to read. It also, again, standardizes the projects languages.
Maintainability: The team mentioned that they are able to do visual testing checking their changes and seeing if they are posted to the database.
Requirements: The team’s code satisfies all requirements.
Build:
3 I could clone the repo, but I had some trouble getting it to run. It gave me an error on the sudo apt-get saying that I cannot run sudo. I tried on the OSU EECS server. I took one point off because the instructions were pretty vague for getting the code to run. Legibility:
4 The code looked great. Variable names were precise. The code was also incredibly clean. It looked very precise and that it had been refactored. It also adhered to Python’s style. Implementation:
4 The team makes good use of functions. The functions abstract out processes. It makes the code easier to follow. The only thing I would recommend is adding some comments to the functions. That will allow people to have a better understanding of what the code does. Maintainability:
4 I did not see any unit tests, but I don’t feel that the team needs any. This script is fairly small and does just a few things. It would be easy to tell if it wasn’t doing the correct thing. They also stated in the code review that they are able to check other sources to make sure the code is working correctly. Requirements:
4 Yes, the code fulfills the requirements.
OK-- so I had some trouble cloning and running this project. I did connect to a VPN according to the instructions in the README file. I believe the README file would be a lot better if it had a little more information about the project itself and how to install and run it on your local machine. However, looking through the two files (scraper.py & generate.py) I could easily understand the implementation of the code because of the way it was written and organized. It seems like this project did not involve a lot of coding, up to this point and that might be the reason I was able to follow it. Building the unit tests for the project was a good idea to make sure the project performs and runs the way it is supposed to.
Build: I was unable to fully run the application(s) as I don’t think I have permissions to download the necessary software on the engineering server. Also, I had trouble telling when commands and scripts did run properly.
Legibility: This is some of the cleanest code I have seen in a long time, and while some variables are named very simply and their purpose was a bit vague, I believe that their simplicity fits in sufficiently with how succinct the scripts need to be. One suggestion would be to add more comments to make the scripts easier to understand.
Implementation: The code is already pretty concise, so the only useful abstractions may be further replacing values in the code with constants so that they can be even more easily configured in the future.
Maintainability: There are no unit tests, but I don’t think they need to be added with this kind of project.
Requirements: Yes, the code fulfills the requirements.
Other: Nothing negative stands out to me in the code, but I am still thoroughly impressed by how concise the scripts are and how they still fulfill all the requirements necessary for the project. Amazing job!
Build: I could clone the repository easily. I could not, however, complete the environment setup for the script. The first line we are told to run for the environment setup contains “sudo”, which our student accounts do not have permission to use on the engineering servers. The next two lines of environment setup worked fine, but the last one failed with the following error: Could not find a version that satisfies the requirement nsx-policy-python-sdk==2.5.1.0.1.15419398 (from -r requirements.txt (line 7)) (from versions: ) No matching distribution found for nsx-policy-python-sdk==2.5.1.0.1.15419398 (from -r requirements.txt (line 7)) This prevented me from preparing all the requirements necessary to run the scripts.
Legibility: Application functionality was split up into folders and files that made sense. All files contained small amounts of code and were easy to follow. Variables were given names that made sense and comments were prevalent. The two separate application functionalities (spreadsheet and tagging) were each given their own readme’s that helped me understand their purposes. Overall, excellent!
Implementation: All functions are very concise and modularized. Work is separated into units that make sense. For example, the “tag” function in the file “tag_op.py” uses the functions “get_vm_id” and “get_tag_association” to tag a virtual machine with appropriate billing info. The longest function (tag) was only 23 lines long, which is pretty concise.
Maintainability: There are no unit tests. Since there are very few functions (or units), I do not believe that unit tests are necessary. It doesn’t seem difficult for the units to be judged on their functionality through observation. All that said, unit tests would not hurt the quality and maintainability of the code. I just believe that they would only give very small returns compared to the amount of time it may take to create them.
Requirements: The code does fulfil the requirements. I do not believe there is much more to be said here. I did not get to run the code myself, but I was present at a demo they gave and it looked like the script was successfully gathering the billing information necessary.
Other: I think that the developers went above and beyond with refactoring their code. I can’t see anything that needs improvement with the code. The environment setup, however, could be updated, as I was unable to follow the procedure that they gave.
Build: Yes. I was able to clone it but not starting it. It was my terminal problem. Isn’t clear on the pre req/requirement of doing the first command
Legibility: It was clear the functionality of each part of the code. It was organized.
Implementation: I am not really familiar with cloud development in python, but the comments help me to understand
Maintainability: I don't think they have unit test.
Requirements: Yes, it follows the guildeline and is well organized.
Other: The instruction in README could be more clear.