Declaração do Problema
Precisa de implementar processos eficazes de code review, estabelecer standards de codificação e criar uma cultura de qualidade que escale com a sua equipa.
Processo de Code Review
┌─────────────┐ ┌─────────────┐ ┌─────────────┐ ┌─────────────┐
│ Feature │───▶│ Self │───▶│ Peer │───▶│ Merge │
│ Development│ │ Review │ │ Review │ │ to Main │
└─────────────┘ └─────────────┘ └─────────────┘ └─────────────┘
│ │
┌──────▼──────┐ ┌──────▼──────┐
│ Automated │ │ Address │
│ Checks │ │ Feedback │
└─────────────┘ └─────────────┘
1. Diretrizes de Code Review
O que Rever
Lógica e Correção
- O código faz o que é suposto fazer?
- Os casos-limite estão contemplados?
- O tratamento de erros é adequado?
- Existem potenciais condições de corrida?
Design e Arquitetura
- Segue os padrões existentes?
- Está devidamente abstraído?
- É testável?
- Mantém a separação de responsabilidades?
Performance
- Existem problemas óbvios de performance?
- As queries à base de dados estão otimizadas?
- Existem loops ou operações desnecessárias?
Segurança
- Existe validação de input?
- Está protegido contra SQL injection?
- Autenticação/autorização corretas?
- Os dados sensíveis são tratados corretamente?
Legibilidade
- Nomes claros?
- Comentários adequados para lógica complexa?
- Formatação consistente?
Modelo de Checklist de Revisão
## Code Review Checklist
### Functionality
- [ ] Code implements the requirements correctly
- [ ] Edge cases are handled
- [ ] Error handling is appropriate
- [ ] No obvious bugs
### Design
- [ ] Follows existing patterns and conventions
- [ ] No unnecessary complexity
- [ ] Proper separation of concerns
- [ ] Changes are backward compatible (if applicable)
### Quality
- [ ] Unit tests added/updated
- [ ] Tests cover edge cases
- [ ] No test code in production
- [ ] Logging is appropriate
### Security
- [ ] Input is validated
- [ ] No sensitive data in logs
- [ ] Authentication/authorization correct
- [ ] No hardcoded credentials
### Performance
- [ ] No N+1 query issues
- [ ] Appropriate caching
- [ ] No unnecessary API calls
- [ ] Resource cleanup (connections, files)
### Documentation
- [ ] README updated if needed
- [ ] API documentation updated
- [ ] Complex logic has comments
2. Quality Gates Automatizados
Pipeline de Qualidade no GitLab CI
stages:
- lint
- test
- security
- build
variables:
COVERAGE_THRESHOLD: 80
lint:
stage: lint
script:
- npm run lint
- npm run format:check
rules:
- if: $CI_PIPELINE_SOURCE == "merge_request_event"
type-check:
stage: lint
script:
- npm run type-check
rules:
- if: $CI_PIPELINE_SOURCE == "merge_request_event"
unit-tests:
stage: test
script:
- npm run test:unit -- --coverage
- |
COVERAGE=$(cat coverage/coverage-summary.json | jq '.total.lines.pct')
if (( $(echo "$COVERAGE < $COVERAGE_THRESHOLD" | bc -l) )); then
echo "Coverage $COVERAGE% is below threshold $COVERAGE_THRESHOLD%"
exit 1
fi
coverage: '/Lines\s*:\s*(\d+\.?\d*)%/'
artifacts:
reports:
coverage_report:
coverage_format: cobertura
path: coverage/cobertura-coverage.xml
integration-tests:
stage: test
services:
- postgres:15
script:
- npm run test:integration
rules:
- if: $CI_PIPELINE_SOURCE == "merge_request_event"
security-scan:
stage: security
script:
- npm audit --audit-level=high
allow_failure: true
Hooks de Pre-commit
# .pre-commit-config.yaml
repos:
- repo: https://github.com/pre-commit/pre-commit-hooks
rev: v4.4.0
hooks:
- id: trailing-whitespace
- id: end-of-file-fixer
- id: check-yaml
- id: check-json
- id: check-merge-conflict
- id: detect-private-key
- repo: https://github.com/psf/black
rev: 23.7.0
hooks:
- id: black
language_version: python3.11
- repo: https://github.com/pre-commit/mirrors-eslint
rev: v8.44.0
hooks:
- id: eslint
files: \.[jt]sx?$
types: [file]
additional_dependencies:
- eslint
- eslint-config-prettier
# Instalar hooks de pre-commit
pip install pre-commit
pre-commit install
3. Standards de Codificação
Exemplos dos Princípios SOLID
Responsabilidade Única
// Mau: A classe faz demasiado
class UserService
{
public function createUser($data) { /* ... */ }
public function sendWelcomeEmail($user) { /* ... */ }
public function generateReport($users) { /* ... */ }
public function exportToCsv($users) { /* ... */ }
}
// Bom: Cada classe tem uma responsabilidade
class UserService
{
public function createUser($data) { /* ... */ }
}
class UserNotificationService
{
public function sendWelcomeEmail($user) { /* ... */ }
}
class UserReportService
{
public function generateReport($users) { /* ... */ }
public function exportToCsv($users) { /* ... */ }
}
Princípio Aberto/Fechado
// Mau: É necessário modificar a classe para adicionar novos métodos de pagamento
class PaymentProcessor
{
public function process($method, $amount)
{
if ($method === 'stripe') {
// Lógica do Stripe
} elseif ($method === 'paypal') {
// Lógica do PayPal
}
// É necessário adicionar mais `elseif` para novos métodos
}
}
// Bom: Aberto à extensão, fechado à modificação
interface PaymentGateway
{
public function charge(float $amount): PaymentResult;
}
class StripeGateway implements PaymentGateway
{
public function charge(float $amount): PaymentResult { /* ... */ }
}
class PayPalGateway implements PaymentGateway
{
public function charge(float $amount): PaymentResult { /* ... */ }
}
class PaymentProcessor
{
public function process(PaymentGateway $gateway, float $amount): PaymentResult
{
return $gateway->charge($amount);
}
}
Inversão de Dependências
// Mau: O módulo de alto nível depende do módulo de baixo nível
class OrderService
{
private MySQLDatabase $db;
public function __construct()
{
$this->db = new MySQLDatabase();
}
}
// Bom: Ambos dependem de uma abstração
interface DatabaseInterface
{
public function query(string $sql): array;
public function insert(string $table, array $data): int;
}
class OrderService
{
public function __construct(
private DatabaseInterface $db
) {}
}
// Pode injetar qualquer implementação
$orderService = new OrderService(new MySQLDatabase());
$orderService = new OrderService(new PostgreSQLDatabase());
$orderService = new OrderService(new InMemoryDatabase()); // Para testes
Standards de Tratamento de Erros
// Definir classes de erro personalizadas
class ApplicationError extends Error {
constructor(message, code, statusCode = 500) {
super(message);
this.name = this.constructor.name;
this.code = code;
this.statusCode = statusCode;
}
}
class ValidationError extends ApplicationError {
constructor(message, field) {
super(message, 'VALIDATION_ERROR', 400);
this.field = field;
}
}
class NotFoundError extends ApplicationError {
constructor(resource) {
super(`${resource} not found`, 'NOT_FOUND', 404);
this.resource = resource;
}
}
// Utilização
async function getUser(id) {
const user = await db.users.findUnique({ where: { id } });
if (!user) {
throw new NotFoundError('User');
}
return user;
}
// Handler global de erros
app.use((err, req, res, next) => {
console.error({
error: err.message,
code: err.code,
stack: err.stack,
requestId: req.id,
});
res.status(err.statusCode || 500).json({
error: {
message: err.message,
code: err.code,
},
});
});
4. Standards de Testes
Estrutura de Testes (Padrão AAA)
describe('OrderService', () => {
describe('createOrder', () => {
it('should create order with valid items', async () => {
// Preparar
const userId = 'user-123';
const items = [
{ productId: 'prod-1', quantity: 2 },
{ productId: 'prod-2', quantity: 1 },
];
const mockProducts = [
{ id: 'prod-1', price: 10.00, stock: 100 },
{ id: 'prod-2', price: 25.00, stock: 50 },
];
productRepository.findByIds.mockResolvedValue(mockProducts);
// Agir
const order = await orderService.createOrder(userId, items);
// Verificar
expect(order.userId).toBe(userId);
expect(order.total).toBe(45.00);
expect(order.items).toHaveLength(2);
expect(productRepository.decrementStock).toHaveBeenCalledTimes(2);
});
it('should throw error when product out of stock', async () => {
// Preparar
const items = [{ productId: 'prod-1', quantity: 10 }];
productRepository.findByIds.mockResolvedValue([
{ id: 'prod-1', price: 10.00, stock: 5 },
]);
// Agir e verificar
await expect(orderService.createOrder('user-1', items))
.rejects
.toThrow('Insufficient stock for product prod-1');
});
});
});
Requisitos de Coverage
// jest.config.js
module.exports = {
coverageThreshold: {
global: {
branches: 80,
functions: 80,
lines: 80,
statements: 80,
},
// Limite mais elevado para código crítico
'./src/services/payment/**/*.js': {
branches: 95,
functions: 95,
lines: 95,
},
},
collectCoverageFrom: [
'src/**/*.{js,ts}',
'!src/**/*.d.ts',
'!src/**/*.test.{js,ts}',
'!src/migrations/**',
],
};
5. Standards de Documentação
Documentação de Código
/**
* Cria uma nova encomenda para o utilizador indicado.
*
* @param userId - O ID do utilizador que está a efetuar a encomenda
* @param items - Array de itens a incluir na encomenda
* @param options - Configuração opcional
* @param options.couponCode - Código de cupão de desconto a aplicar
* @param options.shippingMethod - Método de envio preferido
*
* @returns A encomenda criada com os totais calculados
*
* @throws {ValidationError} Quando o array de itens está vazio
* @throws {NotFoundError} Quando o utilizador ou o produto não é encontrado
* @throws {InsufficientStockError} Quando o stock do produto é insuficiente
*
* @example
* ```typescript
* const order = await orderService.createOrder('user-123', [
* { productId: 'prod-1', quantity: 2 },
* ], {
* couponCode: 'SAVE10',
* });
* ```
*/
async createOrder(
userId: string,
items: OrderItemInput[],
options?: CreateOrderOptions
): Promise<Order> {
// Implementação
}
Documentação de API
# Especificação OpenAPI
openapi: 3.0.0
info:
title: Order API
version: 1.0.0
paths:
/orders:
post:
summary: Create a new order
operationId: createOrder
tags:
- Orders
requestBody:
required: true
content:
application/json:
schema:
type: object
required:
- items
properties:
items:
type: array
items:
$ref: '#/components/schemas/OrderItemInput'
couponCode:
type: string
responses:
'201':
description: Order created successfully
content:
application/json:
schema:
$ref: '#/components/schemas/Order'
'400':
$ref: '#/components/responses/ValidationError'
'404':
$ref: '#/components/responses/NotFoundError'
6. Template de Merge Request
## Description
<!-- What does this MR do? -->
## Related Issue
<!-- Link to related issue: Fixes #123 -->
## Type of Change
- [ ] Bug fix (non-breaking change fixing an issue)
- [ ] New feature (non-breaking change adding functionality)
- [ ] Breaking change (fix or feature causing existing functionality to change)
- [ ] Documentation update
- [ ] Refactoring (no functional changes)
## Checklist
- [ ] My code follows the project's style guidelines
- [ ] I have performed a self-review of my code
- [ ] I have commented my code, particularly in hard-to-understand areas
- [ ] I have made corresponding changes to the documentation
- [ ] My changes generate no new warnings
- [ ] I have added tests that prove my fix is effective or my feature works
- [ ] New and existing unit tests pass locally with my changes
- [ ] Any dependent changes have been merged and published
## Screenshots (if applicable)
<!-- Add screenshots for UI changes -->
## Testing Instructions
<!-- How can reviewers test this change? -->
## Deployment Notes
<!-- Any special deployment requirements? -->
A Mentalidade de Senior: Automatizar o Trivial
Code review é caro. Não desperdice tempo de engenheiros seniores a verificar pontos e vírgulas em falta. Automatize o trivial para que as pessoas se possam focar na arquitetura.
A Filosofia do CI Gate
Se o linter falhar, o Merge Request fica bloqueado. Sem exceções.
- ESLint/Prettier: Estilo e sintaxe.
- SonarQube: Complexidade e bugs.
- Segurança:
npm audit ou Snyk.
Regras de Lint Personalizadas para Arquitetura
Não use apenas regras standard. Escreva regras para a sua arquitetura.
Exemplo: «Não pode importar do módulo frontend para o módulo backend.»
// .eslintrc.js
module.exports = {
rules: {
'no-restricted-imports': ['error', {
patterns: [
{
group: ['../frontend/*'],
message: 'Backend cannot import from frontend module.'
}
]
}]
}
};
Em que as Pessoas se Devem Focar
Quando as pessoas fazem review, procuram: 1. Legibilidade: Consigo perceber isto em 1 minuto? 2. Segurança: O input está sanitizado? 3. Performance: Há um loop dentro de uma query? 4. Test Coverage: Esta correção tem um teste de reprodução?
Picuinhices (espaçamento, nomes de variáveis) → Delegadas aos robôs.
Uma Cultura Saudável de Code Review
Uma cultura saudável de code review é rápida. Ao descarregar a imposição de estilo para o CI, reduz o «Cycle Time» dos Pull Requests e mantém a equipa focada em entregar valor.
Métricas-Alvo:
- Primeira revisão em 4 horas
- MR merged em 24 horas
- Menos de 3 ciclos de revisão, em média
Métricas de Qualidade a Acompanhar
| Métrica | Alvo | Descrição |
|---|
| Code Coverage | >80% | Percentagem de código coberta por testes |
| Complexidade Ciclomática | <10 | Medida da complexidade do código |
| Rácio de Dívida Técnica | <5% | Tempo para corrigir issues vs. tempo de desenvolvimento |
| Duplicação de Código | <3% | Percentagem de código duplicado |
| Tempo de Resposta da Revisão | <4h | Tempo desde a criação do MR até à primeira revisão |
| MR Cycle Time | <24h | Tempo desde a criação até ao merge |
| Taxa de Fuga de Defeitos | <5% | Bugs encontrados em produção vs. testes |
Artigos Relacionados na Wiki