|
|
|
After your project is reviewed by another team, take time to analyze the issues raised, determine if any action needs to be taken, and, if necessary, create the tasks in your issue tracker to address the most important issues identified during the peer review. At the end of iteration 4, you need to submit a "Peer review response" explaining which of the issues raised in the peer-review of your project have been addressed, and why you have decided not to address the remaining issues.
|
|
|
|
-------
|
|
|
|
|
|
|
|
Won't use ModelMapper since it's a lot of work for small improvement. No action
|
|
|
|
|
|
|
|
Yes the docker compose isn't working, we will try to fix it.
|
|
|
|
|
|
|
|
We won't do linux install guide. No action
|
|
|
|
|
|
|
|
Yes the graph line thickness was broken and we fixed it.
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
-------
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
Peer review
|
|
|
|
The project seems to be well structured and understandable. Java Spring framework naming
|
|
|
|
conventions are being followed. TransactionService does a lot of mapping in the backend,
|
|
|
|
maybe a mapping library like ModelMapper could be used to abstract away the mapping
|
|
|
|
logic to a separate mapper object. Maybe it would make the service class easier to read (just
|
|
|
|
an idea, i don’t know if it would actually work for your specific use case)...
|
|
|
|
The installation process was a bit complicated. The docker-compose had some issues, as
|
|
|
|
the compose file referenced to the database port 5434 not 5432 which was the actual docker
|
|
|
|
container port where the service was running. There were also some conflicts between the
|
|
|
|
front-end and back-end communicating. I used docker-compose up to run the project, but
|
|
|
|
the api data did not seem to reach the frontend. The logs showed HTTP 405 not allowed
|
|
|
|
status code. I did not manage to find out the root cause of this issue. The CORS config
|
|
|
|
seemed ok, so I ran out of ideas at the moment. In the end I just used docker-compose to
|
|
|
|
run the backend and database containers and ran the frontend from my local machine with
|
|
|
|
npm. Also it would be nice to have a linux install guide as well. ;D
|
|
|
|
As far as the functionality goes, everything that is described in the release notes is there and
|
|
|
|
working. Only problem I had was with the graph line thickness. It is said that lines that
|
|
|
|
represent transaction amounts larger than 20k euros are thicker, but it did not seem to work
|
|
|
|
for me. I was using firefox as the browser. (5. Cash Flows with Edge Thickness Based on
|
|
|
|
Flow Size #206 (closed)) |
|
|
\ No newline at end of file |
