Как проводишь кросс-ревью?

Как проводить code review (кросс-ревью)

Code review — это критически важный процесс в профессиональной разработке. За 10 лет работы я рецензировал сотни pull request'ов и разработал структурированный подход.

Подготовка к review

Перед тем как начать рецензировать код, нужно подготовиться:

// ЧТО ПРОВЕРИТЬ:
// 1. Прочитать описание PR и задачу
// 2. Посмотреть список изменённых файлов
// 3. Запустить код локально и проверить
// 4. Прочитать все тесты
// 5. Проверить lint и тесты

Этап 1: Общее понимание

Сначала я смотрю на большую картину:

// ВОПРОСЫ:
// - Решает ли это задачу полностью?
// - Не вводит ли это регрессию?
// - Есть ли утечка памяти или race condition?
// - Performance OK?
// - Accessibility OK?
// - TypeScript строго типизирован?

// Смотрю на diff файлов:
const fileChanges = [
  'src/components/Button.tsx',    // изменение компонента
  'src/components/Button.test.tsx' // есть ли тесты?
  'src/styles/button.css',        // стили
];

// Красный флаг: есть .tsx файл, но нет .test.tsx

Этаг 2: Проверка архитектуры

// ПРАВИЛЬНАЯ АРХИТЕКТУРА:

// src/
//   components/
//     Button/
//       Button.tsx          // компонент
//       Button.test.tsx     // тесты (рядом с компонентом)
//       button.module.css   // стили

// НЕПРАВИЛЬНАЯ АРХИТЕКТУРА:
// src/
//   components/Button.tsx
//   tests/ButtonTest.tsx    // тесты в другой папке
//   styles/button.css       // стили в другой папке

// Вопросы архитектуры:
// - Соблюдена ли структура папок?
// - Нарушены ли слои архитектуры?
// - Есть ли круговые зависимости?
// - Компоненты модульные и переиспользуемые?

Этап 3: Проверка кода строка за строкой

Когда я читаю код, ищу следующее:

// 1. ЧИТАЕМОСТЬ И ПОНЯТНОСТЬ
// ПЛОХО: сложная логика в одной строке
const result = items.filter(i => i.active && i.price < 100 && i.stock > 0 && !i.discontinued).map(i => ({...i, discount: i.price * 0.9}));

// ХОРОШО: разбито на понятные шаги
const activeItems = items.filter(item => item.active && !item.discontinued);
const affordableItems = activeItems.filter(item => item.price < 100);
const inStockItems = affordableItems.filter(item => item.stock > 0);
const discountedItems = inStockItems.map(item => ({
  ...item,
  discount: item.price * 0.9
}));

// 2. DRY (Don't Repeat Yourself)
// ПЛОХО: код дублируется
const validateEmail = (email: string) => /^[^@]+@[^@]+\.[^@]+$/.test(email);
const validatePassword = (pwd: string) => /^[^@]+@[^@]+\.[^@]+$/.test(pwd); // копипаста!

// ХОРОШО: выделить в функцию
const isValidFormat = (text: string, pattern: RegExp) => pattern.test(text);
const validateEmail = (email: string) => isValidFormat(email, /^[^@]+@[^@]+\.[^@]+$/);

// 3. SOLID ПРИНЦИПЫ
// Single Responsibility: функция должна делать одно
// Проверяю, не делает ли функция слишком много

// 4. СПЕЦИФИЧНОСТЬ ТИПОВ
// ПЛОХО: any
function processData(data: any) {
  return data.value * 2;
}

// ХОРОШО: точный тип
interface DataItem {
  value: number;
}
function processData(data: DataItem): number {
  return data.value * 2;
}

Этап 4: Проверка тестов

// ВОПРОСЫ К ТЕСТАМ:
// - Есть ли unit тесты?
// - Coverage >= 90%?
// - Тесты покрывают happy path и error cases?
// - Используются ли правильные assertions?

// ПЛОХОЙ ТЕСТ: не проверяет ничего
test('button works', () => {
  render(<Button />);
  // Без assert — это не тест!
});

// ХОРОШИЙ ТЕСТ: проверяет конкретное поведение
test('button displays loading state when disabled', () => {
  render(<Button disabled loading={true} />);

  const button = screen.getByRole('button');
  expect(button).toBeDisabled();
  expect(screen.getByText(/loading/i)).toBeInTheDocument();
});

// ПРОВЕРЯЮ:
// - Используются ли data-testid когда нужно
// - Проверяются ли граничные случаи
// - Есть ли integration тесты

Этап 5: Performance и Accessibility

// PERFORMANCE ПРОВЕРКИ:
// - Ненужные re-renders?
// - Много DOM операций?
// - Lazy loading изображений?
// - CSS classes вместо inline styles?

// ПЛОХО: будет переренден при каждом скролле
function Gallery({ items }) {
  const filtered = items.filter(i => i.category === 'images'); // пересчитывается каждый раз!
  return <div>{filtered.map(...)}</div>;
}

// ХОРОШО: useMemo для оптимизации
function Gallery({ items }) {
  const filtered = useMemo(
    () => items.filter(i => i.category === 'images'),
    [items]
  );
  return <div>{filtered.map(...)}</div>;
}

// ACCESSIBILITY ПРОВЕРКИ:
// - alt текст у изображений?
// - ARIA labels где нужны?
// - Клавиатурная навигация работает?
// - Контрастность достаточна?

// ПЛОХО: нет alt текста
<img src="user-avatar.jpg" />

// ХОРОШО: описательный alt
<img src="user-avatar.jpg" alt="Profile picture of John Doe" />

Этап 6: Security проверки

// ВОПРОСЫ:
// - Нет инъекций XSS?
// - Нет утечек данных в console.log?
// - API ключи не в коде?
// - CORS настроен правильно?

// ПЛОХО: потенциальная XSS уязвимость
<div dangerouslySetInnerHTML={{ __html: userInput }} />

// ХОРОШО: экранировать пользовательский ввод
<div>{userInput}</div> // React автоматически экранирует

// ПЛОХО: API ключ в коде
const API_KEY = 'abc123def456';

// ХОРОШО: переменная окружения
const API_KEY = process.env.REACT_APP_API_KEY;

Как писать комментарии при review

Я использую четырёхуровневую систему:

// УРОВЕНЬ 1: SUGGESTION (предложение)
// 'Может, лучше использовать useMemo для этого?'
// Это мягкое предложение, не критично

// УРОВЕНЬ 2: RECOMMENDATION (рекомендация)
// 'Рекомендую переместить этот код в отдельный хук'
// Это улучшит читаемость

// УРОВЕНЬ 3: IMPORTANT (важно)
// 'Нужно добавить error handling здесь'
// Это влияет на стабильность или security

// УРОВЕНЬ 4: BLOCKER (блокирует)
// 'Это нарушает наш архитектурный слой'
// Это критично и PR не может быть merged

// ПРИ НАПИСАНИИ КОММЕНТАРИЯ:
// 1. Быть конструктивным и уважительным
// 2. Объяснять ПОЧЕМУ, а не просто ЧТО
// 3. Предлагать конкретное решение
// 4. Ссылаться на гайды/документацию если нужно

// ПЛОХОЙ КОММЕНТАРИЙ:
// 'Это плохой код'

// ХОРОШИЙ КОММЕНТАРИЙ:
// 'Эта переменная пересчитывается при каждом render. Предлагаю обернуть в useMemo(), как сделано в src/hooks/useFiltering.ts. Это улучшит performance, особенно при большом количестве элементов.'

Чеклист code review

const reviewChecklist = {
  // ФУНКЦИОНАЛЬНОСТЬ
  completeness: 'Полностью ли решена задача?',
  noRegressions: 'Нет регрессий?',
  errorHandling: 'Обработаны ошибки?',

  // КОД
  readability: 'Код читаемый?',
  dry: 'Нет дублирования?',
  solid: 'Соблюдены SOLID принципы?',
  typeScript: 'Strict TypeScript?',

  // ТЕСТЫ
  unitTests: 'Unit тесты есть?',
  coverage: 'Coverage >= 90%?',
  edgeCases: 'Covered edge cases?',

  // PERFORMANCE & QUALITY
  performance: 'Performance OK?',
  accessibility: 'A11y OK?',
  security: 'Security OK?',
  mobile: 'Mobile responsive?',

  // ДОКУМЕНТАЦИЯ
  comments: 'Сложный код задокументирован?',
  types: 'TypeScript типы правильные?',

  // ПРОЦЕСС
  lint: 'npm run lint проходит?',
  tests: 'npm run test проходит?',
  build: 'npm run build проходит?',
};

// Если ВСЕ пункты ✓ — approve PR
// Если есть blockers — request changes
// Если есть suggestions — comment

Примеры хороших и плохих PR'ов

ПЛОХОЙ PR:

• Изменяет 50+ файлов
• Нет tests
• Описание: 'fixed stuff'
• Код не компилируется
• Результат: Reject

ХОРОШИЙ PR:

• 3-5 логически связанных файлов
• Coverage 95%+
• Понятное описание и линк на задачу
• Все тесты проходят
• Результат: Approve

Как принимать feedback при review

Когда тебе рецензируют код:

// ПРАВИЛЬНОЕ ОТНОШЕНИЕ:
// 'Спасибо за feedback, я исправлю'
// 'Хорошее предложение, буду помнить'
// 'Согласен, переделаю'

// НЕПРАВИЛЬНОЕ ОТНОШЕНИЕ:
// 'Но это работает же!'
// 'На мой взгляд, это ок'
// 'Зачем это надо менять?'

// ЛУЧШАЯ ПРАКТИКА:
// Если disagree — обсуди и приведи аргументы
// Если agree — исправь и спасибо скажи
// Если не знаешь — спроси и учись

Итог

Code review это не про критику, а про:

• Разделение знаний в команде
• Повышение качества кода
• Обучение друг друга
• Предотвращение багов

Когда я делаю review, я помню что на другом конце сидит человек, который потратил часы на этот код. Моя работа — помочь ему улучшить его работу, а не раскритиковать.