GoMFT/docs/docs/development/code-review.md

sidebar_position, title

sidebar_position	title
4	Code Review Guidelines

Code Review Guidelines

This document outlines the code review process and expectations for the GoMFT project.

Purpose of Code Reviews

Code reviews serve several important purposes:

• Ensuring code quality and consistency
• Identifying bugs, edge cases, and potential issues early
• Sharing knowledge among team members
• Ensuring adherence to project standards and best practices
• Validating that the implementation meets requirements

Code Review Process

1. Before Requesting a Review

Before requesting a review, ensure your code:

• Passes all automated tests
• Follows the project's coding standards
• Is well-documented
• Includes appropriate tests
• Has a clear, descriptive PR title and description

2. Requesting a Review

• Create a pull request against the appropriate branch
• Fill out the PR template completely
• Tag appropriate reviewers based on the code being changed
• Respond to any automated CI/CD feedback

3. Conducting a Review

When reviewing code, focus on:

Code Quality

• Is the code readable and maintainable?
• Does it follow project conventions and patterns?
• Is the implementation efficient?
• Are edge cases handled appropriately?

Functionality

• Does the code meet the requirements?
• Does it handle error conditions properly?
• Is the user experience considered?

Testing

• Are there sufficient tests?
• Do tests cover edge cases?
• Are tests reliable (not flaky)?

Security

• Are there potential security issues?
• Is user input properly validated?
• Are credentials or sensitive data handled securely?

Documentation

• Is the code well-documented?
• Are public APIs clearly documented?
• Is the documentation accurate and up-to-date?

4. Providing Feedback

When providing feedback:

• Be specific and clear
• Offer suggestions for improvement
• Differentiate between required changes and optional suggestions
• Provide context or reasoning for requested changes
• Be constructive and respectful

Use these comment prefixes to indicate the severity of feedback:

• Blocker: Must be addressed before merging
• Suggestion: Recommended improvement, but not required
• Question: Request for clarification
• Nitpick: Minor style or formatting issue
• Praise: Highlight particularly good code

5. Responding to Feedback

When receiving review feedback:

• Address all comments
• Explain your reasoning if you disagree with a suggestion
• Ask for clarification if needed
• Thank reviewers for their input
• Mark resolved comments as such

6. Approving and Merging

A PR can be merged when:

• It has received approval from at least one reviewer
• All "Blocker" issues are resolved
• All automated checks are passing
• The PR has been rebased on the latest target branch

Best Practices for Reviewers

Focus on the Important Things

• Prioritize correctness, security, and maintainability
• Don't get too caught up in style issues that could be automated
• Consider the big picture and overall architecture

Be Timely

• Try to review PRs within 1-2 business days
• If you can't review promptly, let the author know or reassign
• For urgent fixes, prioritize those reviews

Be Thorough

• Take the time to understand the code
• Test the code locally if necessary
• Consider edge cases and failure modes

Be Respectful

• Focus on the code, not the person
• Phrase feedback as suggestions or questions
• Acknowledge good work and improvements

Best Practices for Authors

Keep PRs Focused

• Each PR should address a single concern
• Large changes should be broken into smaller, logical PRs
• Avoid unrelated changes in a PR

Provide Context

• Explain the purpose and approach in the PR description
• Link to relevant issues or documentation
• Point out areas where you're uncertain or would like specific feedback

Respond Promptly

• Address feedback in a timely manner
• Ask questions if feedback is unclear
• Be open to suggestions and alternatives

Test Thoroughly

• Test your changes locally before requesting review
• Consider edge cases and error scenarios
• Update tests to cover new functionality

Special Considerations

Security-Related Changes

• Security-focused changes require extra scrutiny
• At least one reviewer should have security expertise
• Consider potential attack vectors and edge cases

API Changes

• Changes to public APIs require careful review
• Consider backward compatibility
• Ensure API changes are well-documented

Database Changes

• Review for potential performance issues
• Consider migration strategy and backward compatibility
• Validate data integrity considerations

UI Changes

• Consider accessibility implications
• Review for consistency with design standards
• Test on different devices and screen sizes

Learning from Code Reviews

Code reviews are a learning opportunity:

• Take note of recurring feedback to improve future code
• Share knowledge gained from reviews with the team
• Use reviews to identify areas where documentation or guides could be improved

Code Review Checklist

General

• Code is well-structured and follows project patterns
• Variables and functions have clear, descriptive names
• Comments explain "why" not just "what"
• Unnecessary code is removed (commented code, debug logs)
• No hardcoded values that should be configurable

Go-Specific

• Error handling is appropriate and consistent
• Follows Go idioms and best practices
• Concurrent code is safe and efficient
• Uses appropriate Go standard library functions
• Properly handles resources (file handles, connections)

Frontend-Specific

• UI is responsive and accessible
• HTMX usage follows project patterns
• Templ templates are clean and maintainable
• JavaScript is minimal and follows best practices
• CSS follows project conventions

Testing

• Tests are included for new functionality
• Tests cover edge cases and error paths
• Tests are clear and maintainable
• Mocks and fixtures are used appropriately

Security

• Input validation is thorough
• No SQL injection vulnerabilities
• Authentication and authorization are properly implemented
• Sensitive data is handled securely

Performance

• Code is efficient for expected scale
• Database queries are optimized
• Appropriate caching is used
• Resources are used efficiently

Remember that code reviews are a collaborative process aimed at improving the overall quality of the codebase. Both reviewers and authors should approach the process with a growth mindset and mutual respect.