Skip to content
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

Tranjy cr2 #5

Open
wants to merge 4 commits into
base: development
Choose a base branch
from
Open

Tranjy cr2 #5

wants to merge 4 commits into from

Conversation

jtran549
Copy link
Collaborator

@jtran549 jtran549 commented Nov 8, 2020

Analysis
This application is used to document time spent on different projects with different clients. The client side page features input options for a Client, a Project, and the time spent on said project. Along with the inputs, it appears there is a display feature that is meant to display a list of projects and time spent on each project.

Was the program available in UC Github on time?
Yes

Did the program compile?
Yes

Is the program easy for you, as an outsider, to understand?
Yes

Three technical concepts learned

  • When using a th:href tag, there are different ways to specify the url that you want to navigate to. This was learned when I was making a change and reading through the thymeleaf documentation
  • You do not need to specify the xmlns: tag in the top of a fragment, since when thymeleaf renders the fragment into the specific page, that page already has that reference at the top
    -Not only can you tie an object to an attribute in the controller, you can also tie a list of objects to iterate through
    image

@discospiff
Copy link

There's quite a bit of new functionality introduced in this review... which I normally don't recommend as part of a code review. However, this functionality appears to make quite a bit of progress on the team's features, and there are good code review tips as well. I recommend that the team review this branch closely, make any additional changes they want to make, and then consider merging.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants