Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Code review #183

Closed
busebuz opened this issue Dec 14, 2015 · 3 comments
Closed

Code review #183

busebuz opened this issue Dec 14, 2015 · 3 comments
Assignees

Comments

@busebuz
Copy link
Contributor

busebuz commented Dec 14, 2015

My create-violation html code needs to be reviewed. Can someone handle it?

@atalayyirik
Copy link
Contributor

Yes I can do it

@atalayyirik atalayyirik self-assigned this Dec 14, 2015
@atalayyirik
Copy link
Contributor

Code Clarity
The code looks quite clear.The design is developed well.

Bugs
There is not a main bug for the page to crush or not to be worked but there are slight bugs that are needed to be fixed as I found using IntelliJ.These bugs are mostly typos and it does not require major developments to fix.Besides, there is one problem of one redundant closing tag.I think you should easily fix it by just erasing the extra tag.

Missing Features
It does not need any new feature.It meets our requirements.

Coding Convention
Variables
There is no variable in the page.
Indentation
The code structure is well.It is easy to follow up step by step.
Comments
There are some comments of basic tags and code.They can be extended to other tags so that other people can understand which tag is for what purpose.

Tests and Documentation
There should be unit tests to check filling fields properly.

Modification
Because I know and do frontend part together with my team and the code is well written to follow up I can easily modify the code.For instance I can easily modify the fields and its types so that they can be in another report of something else.

She used nested loops to view page.I think nested loops are efficient to show something without using the same code everytime.It is both efficient and easier once you get familiar with it.

@busebuz
Copy link
Contributor Author

busebuz commented Dec 14, 2015

Thank you Atalay! I've fixed it :)

@busebuz busebuz closed this as completed Dec 14, 2015
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants