Было ли в команде Code Review

Code Review в команде: важность, процесс и best practices

Code Review — это процесс, при котором опытные разработчики проверяют код, написанный коллегами, перед его добавлением в main ветку. Это одна из наиболее эффективных практик для улучшения качества кода и знаний команды.

Зачем Code Review нужен

1. Поиск ошибок и баги

// Автор написал это:
function processUsers(users) {
  let processed = [];
  for (let i = 0; i < users.length; i++) {
    // Ошибка: забыл return
    users[i].processedAt = new Date();
    processed.push(users[i]);
  }
  return processed;
}

// Reviewer видит проблему:
// 1. Функция модифицирует исходный массив (side effect)
// 2. Не обрабатывает null/undefined
// 3. Можно упростить используя .map()

// Предложенный fix:
function processUsers(users: User[]): User[] {
  return users.map(user => ({
    ...user,
    processedAt: new Date()
  }));
}

2. Knowledge sharing (распространение знаний)

// Junior разработчик написал:
const userId = useSelector(state => state.user.id);
const userName = useSelector(state => state.user.name);
const userEmail = useSelector(state => state.user.email);

// Reviewer показывает лучший подход:
const user = useSelector(state => state.user);
const { id, name, email } = user;

// Или ещё лучше (с мемоизацией):
const user = useSelector(
  state => state.user,
  shallowEqual // Избегаем лишних re-renders
);

3. Стандартизация кода

// Отсутствие Code Review ведёт к беспорядку:
// - Разные стили кодирования
// - Разные подходы к error handling
// - Разные структуры папок
// - Разные названия переменных

// Code Review гарантирует:
// - Единый style guide
// - Consistent patterns
// - Понятный код для всей команды

4. Менторство и онбординг

// Junior задаёт вопросы в PR:
// "Почему ты использовал useCallback вместо useMemo?"

// Senior объясняет:
// useCallback возвращает функцию, useMemo возвращает значение
// useCallback нужен для функций, передаваемых как dependencies
// useMemo для дорогих вычислений

// Результат: Junior учится, вся команда видит обсуждение

Процесс Code Review

Шаг 1: Автор открывает Pull Request

## Description
Имплементирована фича авторизации через JWT tokens

## Type of change
- [x] Bug fix
- [ ] New feature
- [ ] Breaking change
- [x] This change requires a documentation update

## How Has This Been Tested
- Unit tests написаны
- E2E тест с Playwright
- Протестировано на staging

## Checklist
- [x] My code follows the code style
- [x] Tests added/updated
- [x] Documentation updated
- [x] No console errors
- [x] Lighthouse score > 90

Шаг 2: Reviewer проверяет PR

Reviwer смотрит:

// 1. Логика и корректность
function authenticateUser(credentials) {
  // Проверяет: обработаны ли все edge cases?
  // Есть ли security issues?
  // Правильно ли обработаны ошибки?
}

// 2. Performance
// Нет ли лишних re-renders?
// Нет ли N+1 queries?
// Хороший ли размер bundle?

// 3. Maintainability
// Понятны ли названия переменных?
// Есть ли комментарии где нужны?
// Нет ли дублирования кода?

// 4. Testing
// Достаточно ли тестов?
// Покрывают ли они edge cases?

// 5. Documentation
// Обновлена ли документация?
// Есть ли примеры использования?

Шаг 3: Reviewer оставляет комментарии

## Changes requested

### Line 42: Missing error handling
```javascript

await fetchUserData(id); // Что если запрос упадёт?

Predefined:

try {
  const data = await fetchUserData(id);
  return data;
} catch (error) {
  logger.error('Failed to fetch user', { userId: id, error });
  throw new UserNotFoundError('User not found');

}

Line 127: Performance issue

Этот компонент re-renders на каждый render родителя. Используй React.memo() или useMemo().

General: Nice implementation!

Мне нравится как ты упростил логику авторизации.

#### Шаг 4: Автор делает исправления

```javascript
// Автор применяет feedback:
function authenticateUser(credentials) {
  try {
    const token = verifyCredentials(credentials);
    return { token, expiresIn: 3600 };
  } catch (error) {
    if (error instanceof InvalidCredentialsError) {
      throw new Error('Invalid email or password');
    }
    logger.error('Unexpected auth error', error);
    throw error;
  }
}

// Делает commit с сообщением о fix
// "Fix: Add proper error handling in authenticateUser"

Шаг 5: Approval и merge

## Approved by @senior-dev

Отличная работа! Все feedback учтены.
Rdy to merge.

Как проводить хороший Code Review

DO's (хорошие практики)

## Хороший отзыв:

### Question (задавай вопросы вместо критики)
"Почему мы используем `const` здесь? Будут ли случаи, когда нужна переменная?"

### Compliment (похвали хороший код)
"Отличный способ использовать Optional Chaining здесь!"

### Explain (объясни почему)
"Мы используем `useCallback` потому что функция передаётся как dependency в другой hook.
Без useCallback он создавался бы при каждом render и вызовет лишний effect."

### Suggest (предложи улучшение)
"Можно ли упростить это используя `.map()` вместо `.forEach()`?"

### Context (дай контекст)
"В соответствии с нашим style guide (docs/style-guide.md),
компоненты должны быть в PascalCase."

DON'Ts (плохие практики)

## Плохой отзыв:

### Criticism без помощи
"Это неправильно" <- не помогает, обидит человека

### Personal attacks
"Ты явно не знаешь React" <- токсично

### Off-topic
"Тебе нужно больше спать" <- не дело code review

### Nitpicking
"Эта переменная должна быть userIdFromDatabase вместо id" <- мелочь

### Требование "как я бы написал"
"Я бы написал это вот так [свой код]" <- у каждого свой стиль

Инструменты для Code Review

// GitHub Pull Requests (самый популярный)
// - Встроен в GitHub
// - Comments, suggestions, approvals
// - Интеграция с CI/CD
// - Ограничение: только для GitHub

// GitLab Merge Requests
// - Встроен в GitLab
// - Похож на GitHub PR
// - Более гибкий CI/CD

// Gerrit (Google Code Review)
// - Более строгий процесс
// - Требует approval перед merge
// - Хорошо для больших команд

// Smartbear CodeCollaborator
// - Standalone tool
// - Интеграция с разными VCS
// - Comprehensive reporting

Метрики Code Review

// Хорошие метрики:
const codeReviewMetrics = {
  "Среднее время на review": "4-8 часов", // Должно быть быстро
  "Среднее количество комментариев": "3-5", // Должно быть критичное
  "% PR с замечаниями": "40-60%", // Это нормально
  "Количество reviewers на PR": "1-2", // Баланс между qualitity и speed
  "Повторяемость ошибок": "<5%" // Если часто одинаковые ошибки, нужен style guide
};

// Плохие метрики:
const badMetrics = {
  "Код идёт в main без review": true, // Это очень плохо
  "Никто не комментирует PR": true, // Никто не проверяет качество
  "Review занимает несколько дней": true, // Блокирует разработку
  "Все PR утверждаются на автомате": true // Review бесполезен
};

Реальный пример Code Review

## PR: Add user authentication with JWT

### Author: @junior-dev

---

### Reviewer: @senior-dev

#### Comment 1: Security Issue (BLOCKER)
Line 15: Вы отправляете raw password в логи

```typescript
logger.info('User login attempt', { username, password }); // ОПАСНО!

Предложение:

logger.info('User login attempt', { username, timestamp: new Date() });
logger.debug('Raw data', { password }); // Только на dev уровне

Зачем: пароль в логах = скомпрометирован аккаунт

---

Comment 2: Code Style

Line 42: Соответствует ли TypeScript strict mode?

const user = getUserFromDB(id); // Тип неизвестен

Предложение:

const user: User | null = await getUserFromDB(id); if (!user) {

  throw new UserNotFoundError();

}

---

Comment 3: Testing

Мне не хватает unit-тестов для edge cases:

• Что если пользователь заблокирован?
• Что если пароль пуст?
• Что если БД недоступна?

---

Comment 4: Compliment

Отличный способ использовать try-catch и custom errors!

---

Decision: CHANGES REQUESTED

Ожидаем fixes перед merge.

### Culture and Team Dynamics

```javascript
// Code Review должна быть:
// - Constructive (строительная, не деструктивная)
// - Respectful (вежливая и уважительная)
// - Collaborative (совместная, не конфликтная)
// - Educational (обучающая, знания распространяются)

// Как создать хорошую culture:
const culture = {
  "Фокус на код, не на человека": true,
  "Благодарить за feedback": true,
  "Вопрос вместо требования": true,
  "Признание ошибок": true,
  "Периодический retro": true,
  "Праздновать улучшения": true
};

Итог

Code Review — это не про критику, это про:

• Контроль качества
• Распространение знаний
• Улучшение команды
• Развитие junior разработчиков
• Долгосрочная health проекта

Хороший Code Review:

• Занимает 30-60 минут
• Конструктивен и вежлив
• Ловит реальные проблемы (не мелочь)
• Помогает автору учиться
• Улучшает качество всего кода

Процесс работает когда:

• Все видят code review как обязательный
• Нет спешки (выделяется время)
• Все воспринимают feedback позитивно
• Обратная связь своевременна
• Есть документированные стандарты

Если в команде есть хороший Code Review — качество кода естественно растёт, а знания равномерно распределяются.