Как было организовано code review на прошлой работе

# Как было организовано code review на прошлой работе

Это поведенческий вопрос, направленный на понимание вашего опыта работы в команде и качества кода. Вот типичный ответ:

Мой опыт с Code Review

На моей последней работе в компании из 30 разработчиков code review был ключевой частью процесса разработки.

Процесс Code Review

1. Инструменты

GitHub / GitLab:

• Pull Requests (PR) вместо прямых коммитов в main
• Любой код должен был быть в отдельной feature branch
• Minimal: один одобренный review перед merge

Комментарии в PR:

• Inline comments для конкретных строк
• Suggestion mode для предложения изменений
• Обсуждение архитектурных решений

2. Кто делает review?

- Code owner (обычно Lead/Senior dev)
  └─> Проверяет архитектуру, безопасность

- 2+ developer из команды
  └─> Проверяют логику, best practices

- Автоматические checks (CI/CD):
  └─> SonarQube (качество кода)
  └─> JUnit тесты (coverage > 80%)
  └─> Checkstyle (стиль кода)
  └─> Linter'ы (статический анализ)

3. Требования для merge'а

✅ Минимум 2 одобрения от разработчиков ✅ Code owner одобрил ✅ Все комментарии решены ✅ Все CI checks пройдены (tests, lint, coverage) ✅ Нет конфликтов с main branch ✅ Commit message следует стандартному формату ✅ Тесты покрывают > 80% нового кода

4. Чеклист при review

Мы использовали github issue template:

## Type of change
- [ ] Bug fix
- [ ] New feature  
- [ ] Breaking change

## Description
Краткое описание что было изменено

## Testing
- [ ] Added unit tests
- [ ] Added integration tests
- [ ] Manual testing done

## Code Quality
- [ ] Followed code style guide
- [ ] Code coverage > 80%
- [ ] No hardcoded values
- [ ] No TODO/FIXME comments
- [ ] Proper error handling
- [ ] Logging added where needed

## Documentation
- [ ] Updated README if needed
- [ ] Added/updated javadocs
- [ ] Updated API documentation

## Checklist
- [ ] My code follows the style guidelines
- [ ] I have performed a self-review
- [ ] I have commented complex code
- [ ] No new warnings generated
- [ ] Tests pass locally

5. Типичные замечания при review

❌ Architecture issues:

// Reviewer comment: This violates layered architecture.
// Please move database access to repository, not in controller.

@PostMapping("/users")
public User createUser(@RequestBody UserDTO dto) {
    // ❌ Прямой доступ к БД в контроллере
    entityManager.persist(new User(dto.getName()));
    return ...;
}

// ✅ Правильно:
@PostMapping("/users")
public User createUser(@RequestBody UserDTO dto) {
    // Контроллер вызывает сервис
    return userService.createUser(dto);
}

❌ Missing error handling:

// Reviewer: Add try-catch or throws declaration

public List<User> getAllUsers() {
    return userRepository.findAll(); // Что если БД упала?
}

// ✅ Better:
public List<User> getAllUsers() throws UserServiceException {
    try {
        return userRepository.findAll();
    } catch (DataAccessException e) {
        throw new UserServiceException("Failed to fetch users", e);
    }
}

❌ Missing tests:

// Reviewer: Coverage dropped from 85% to 72%.
// Please add unit tests for new logic.

❌ Code style:

// Reviewer: Variable names should be descriptive

List<String> x = userRepository.findAll(); // ❌ Плохо
List<String> userNames = userRepository.findAll(); // ✅ Хорошо

❌ Performance:

// Reviewer: N+1 query problem. Consider using join fetch.

public List<Order> getOrders() {
    return orderRepository.findAll();
    // Это вызовет 1 запрос за order + N запросов за customers
}

// ✅ Better:
@Query("SELECT DISTINCT o FROM Order o LEFT JOIN FETCH o.customer")
public List<Order> getOrders();

❌ Security:

// Reviewer: SQL injection vulnerability. Use parameterized queries.

String query = "SELECT * FROM users WHERE id = " + userId; // ❌
Query q = em.createQuery("SELECT u FROM User u WHERE u.id = :id") // ✅
    .setParameter("id", userId);

6. Сроки review

Наш процесс:

• Developer создает PR
• SLA на first review: 24 часа (обычно 1-2 часа)
• После комментариев: 12 часов на ответ
• Всего в process: 2-3 дня максимум

Slack notification:

[GitHub] @john created PR #1234: Add user authentication
Reviewers: @alice, @bob

После review:
[GitHub] @alice approved #1234
[GitHub] @bob requested changes in #1234

7. Tools мы использовали

8. CI/CD Pipeline

name: CI/CD

on: [pull_request, push]

jobs:
  build-and-test:
    runs-on: ubuntu-latest
    
    steps:
      - uses: actions/checkout@v2
      
      - name: Set up JDK
        uses: actions/setup-java@v2
        with:
          java-version: '17'
      
      - name: Build and test
        run: mvn clean package
      
      - name: Code coverage
        run: mvn jacoco:report
      
      - name: SonarQube analysis
        run: mvn sonar:sonar
      
      - name: Lint
        run: mvn checkstyle:check

9. Feedback культура

Правила code review:

• Конструктивные комментарии, не критика личности
• Принимать критику как помощь, не как нападение
• Не стесняться просить объяснение
• Хвалить хороший код, не только критиковать

Пример плохого комментария:

❌ "Это ужасный код. Кто это написал?"

Пример хорошего комментария:

✅ "Этот подход работает, но рекомендую использовать Stream API
    для лучшей читаемости. Вот пример:
    list.stream().filter(...).map(...).collect(...)"

10. Learning и Knowledge Sharing

После сложных review'ов проводили:

• Техсеминары (1 раз в неделю) — обсуждение архитектурных паттернов
• Code Review Academy — обучение Junior'ов best practices
• Документация — в README и Wiki описывали conventions
• Пост-мортем — разбор критических ошибок

Что я бы улучшил

1. Автоматизировать больше checks (например, coverage requirements)
2. Установить stricter deadline'ы на merge'и
3. Создать более структурированный feedback template
4. Добавить metrics (average review time, comments per PR)
5. Использовать код review tools типа Gerrit для более deep analysis

Типичный ответ на собеседовании

"На моей последней работе code review был обязательным процессом:

1. Любой код в отдельной branch и Pull Request
2. Минимум 2 одобрения до merge'а
3. Все CI checks должны пройти (tests, lint, coverage > 80%)
4. SonarQube для статического анализа
5. Архитектуру проверял Team Lead/Code Owner
6. Логику проверяли 2+ разработчика

Я научился:
- Писать читаемый, maintainable код
- Принимать конструктивную критику
- Помогать junior разработчикам
- Понимать importance code quality

В процессе я был активным reviewer'ом, помогая команде
мейнтейнить high quality standards. Обычно я делал 5-10
code review'ов в неделю, помогая коллегам улучшать их код."

Заключение

Code Review — это не контроль, а: ✅ Инвестиция в качество кода ✅ Knowledge sharing в команде ✅ Профилактика багов до production ✅ Обучение и развитие разработчиков

Хороший code review culture повышает качество продукта на 40-60%!