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

Code review #182

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

Code review #182

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

Comments

@zmelce
Copy link
Contributor

zmelce commented Dec 14, 2015

Can you review my code considering certain criteria

@busebuz
Copy link
Contributor

busebuz commented Dec 14, 2015

Code Clarity
I also am dealing with frontend so code looks quite familiar and clear. Your indentations and code style is really accurate and easy to follow.

Bugs
There isn't any bug however there is just a minor issue that analyzing your code by using Intellij Code Analyzer, I found some typo warnings. They are basically because of the violation report it is not written in English, but well it is not a problem.

However, in Intellij. I received a warning there is a tag named (on line 62) and it is unknown. Maybe you can fix it. Also there is some syntax problem on line 66 and 166. I think you should check them.

Missing Features
It meets our requirements. Features were supposed to be done in this commit that are done.

Coding Convention
 Variable Names
There are no variables but all tags are used properly

 Indentation
I can follow every tag so indentation is good and organized.

 Comments
Your code is really clear. But I'm also a member of frontend team so maybe comments can be more useful at this point for other people. You can add more comment explaining what all the tags are for.

Tests and documentation
Unit tests should be done

Modification
Someone familiar with html part of this project can easily understand and modify this code. However, there would be people in our team who are not familiar with html, so adding comments can be useful. Yet, I can easily modify your code by tags that you used.

Your code is really well-written and clear. I think when you fix little errors and add comments it will be perfect! Thank you for your hard working.

@zmelce
Copy link
Contributor Author

zmelce commented Dec 14, 2015

Hi Buse,
thank you for reviewing. I will consider your evaluation but need name of code pages and commit id of line numbers that you remarked to fix bugs.

@busebuz
Copy link
Contributor

busebuz commented Dec 14, 2015

p.s The file I reviewed was: viewViolation.html with commit id: 85809ac

@busebuz busebuz closed this as completed Jan 7, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants