Brad-Davidson / MealPlanner

0 stars 1 forks source link

Code review 1 Tyler Hitchner #25

Closed Hitchnt closed 3 years ago

Hitchnt commented 3 years ago

Disclaimer I saw that their already 2 reviews already, so I try not restate what was already stated like your circleCI not working and others, but instead try to look for something new that can help.

  1. Analysis of the program.

The program ran on end with not problems. at first glances without reading the readme I was able to follow and understand what the program is supposed to accomplished. Afterwards I did go to the readme to review that the step are and based of the service and the dataclass that already populate I see that your set your set to go and complete those next.

  1. Was the program available in UC Github on time? Yes

  2. Is the program documented/commented well enough for you to understand? Yes, not really any loose ends, though I did see that you have big chunk of code commented out in service MealPlanService class but I can tell that you working on the part and due some reason has to comment it out.

  3. Does the program compile? Yes

  4. Rationale behind your changes. Just simple removing import or code to keep it clean while continue to complete project will working in a cleaner environment Add image button due to not being able read small text on my screen without blowing them out, but also show what look like with using images instead of text to communicate with the user.

  5. Specific technical concepts that I learned.

    • 1 This first I seen the using query the in android/Kotlin so it interesting to see it for the first time
    • In the unit test This first I have seen date being using while I may use currently it did open my eyes for later use
    • Got to see how Date() and setting the time works when dealing with dates.
discospiff commented 3 years ago

Many really good suggestions here. The team should discuss and consider merging this branch.