Conventional Comments: как перестать бомбить друг на друга в ревью

❌ Это плохая реализация
❌ Переделай по..
❌ Не так сделалл
❌ Почему ты не использовал паттерн НОУНЕЙМ?

✅ praise: Огонь валидация! Учел все edge cases 🔥
✅ suggestion (non-blocking): Можно вынести валидацию в отдельный класс. Не критично, можно в следующем PR.
✅ issue (blocking): Тут SQL-инъекция в поле "имя". Надо пофиксить перед мержем.
✅ question: Почему bcrypt, а не argon2? Просто интересно.
✅ nitpick: Опечатка "валидатция" → "валидация"

<ярлык> [важность]: что не так

[опционально: как исправить]

suggestion (non-blocking): Можно короче через array_map

$result = array_map(fn($item) => $item * 2, $array);

Но если привык к foreach, оставь как есть.

praise: О, хорошее название класса. Сразу все понятно.

praise: Крутой рефакторинг! Код стал в 2 раза короче.

issue (blocking): SQL-инъекция, братан

Используй prepared statements:
$stmt = $pdo->prepare("SELECT * FROM users WHERE id = ?");
$stmt->execute([$_GET['id']]);

issue (blocking): Деление на ноль, если $count === 0

Добавь проверку перед делением.

suggestion (non-blocking): Можно вынести в отдельный метод

Будет проще тестировать. Но если времени нет, забей.

suggestion: Добавь кеширование для этого запроса

Он дергается на каждой странице, а данные меняются раз в час.
Redis решит проблему.

question: Почему setTimeout вместо setInterval?

Просто хочу понять логику.

question: Зачем тут try-catch, если ошибка все равно летит дальше?

try {
    $result = $this->api->call();
} catch (Exception $e) {
    throw $e; // <- зачем?
}

nitpick: Лишний пробел в конце строки

nitpick: Можно переименовать $data в $userData

nitpick: Опечатка в комментарии

public function processOrder($orderId) {
    $order = Order::find($orderId);

    if ($order->status !== 'pending') {
        throw new Exception('Order not pending');
    }

    $user = User::find($order->user_id);

    if ($user->balance < $order->total) {
        throw new Exception('Insufficient balance');
    }

    $user->balance -= $order->total;
    $user->save();

    $order->status = 'paid';
    $order->save();

    Mail::to($user->email)->send(new OrderPaid($order));

    foreach ($order->items as $item) {
        $product = Product::find($item->product_id);
        $product->stock -= $item->quantity;
        $product->save();
    }

    return $order;
}

praise: Хорошо обработал все шаги оплаты.

---

issue (blocking): Нет транзакции — если упадет на середине, 
деньги спишутся, а заказ не обновится

DB::transaction(function () use ($orderId) {
    // вся логика здесь
});

---

suggestion (non-blocking): Метод делает слишком много

Можно разбить на validateOrder(), chargeUser(), updateStock().
Проще тестировать и переиспользовать.

Но можно в следующем PR, если есть время.

---

question: Почему письмо внутри транзакции?

Если письмо долго отправляется, транзакция висит.
Обычно такое через очередь делают.

---

nitpick: N+1 запросов в цикле

$order->load('items.product') решит проблему.

async upload(req, res) {
  const file = req.file;

  if (!file) {
    return res.status(400).json({ error: 'No file' });
  }

  const allowedTypes = ['image/jpeg', 'image/png'];
  if (!allowedTypes.includes(file.mimetype)) {
    return res.status(400).json({ error: 'Invalid type' });
  }

  if (file.size > 5 * 1024 * 1024) {
    return res.status(400).json({ error: 'File too large' });
  }

  const filename = `${Date.now()}-${file.originalname}`;
  const path = `uploads/avatars/${filename}`;

  await fs.writeFile(path, file.buffer);

  await User.update(
    { avatar: filename },
    { where: { id: req.user.id } }
  );

  res.json({ avatar: filename });
}

praise: Отличная валидация типов и размера! 👏

---

issue (blocking): Path Traversal уязвимость

Если пользователь загрузит "../../etc/passwd", 
он может перезаписать системные файлы.

Используй UUID вместо оригинального имени:

const { v4: uuid } = require('uuid');
const ext = path.extname(file.originalname);
const filename = `${uuid()}${ext}`;

---

suggestion (blocking): Нужно удалять старый аватар

Сейчас старые файлы остаются и занимают место.

const user = await User.findByPk(req.user.id);
if (user.avatar) {
  await fs.unlink(`uploads/avatars/${user.avatar}`);
}

---

suggestion (non-blocking): Можно добавить resize до 200x200

Библиотека sharp:

await sharp(file.buffer)
  .resize(200, 200)
  .webp({ quality: 80 })
  .toFile(path);

Но это можно в следующем PR.

---

nitpick: Магическое число 5 * 1024 * 1024

const MAX_AVATAR_SIZE = 5 * 1024 * 1024; // 5 MB

{
  "CC Suggestion": {
    "prefix": "ccs",
    "body": "suggestion (non-blocking): $1\n\n$2"
  },
  "CC Issue": {
    "prefix": "cci",
    "body": "issue (blocking): $1\n\n$2"
  },
  "CC Praise": {
    "prefix": "ccp",
    "body": "praise: $1"
  }
}

praise: Отлично обработал edge cases!
suggestion (non-blocking): Можно добавить кеширование.
Если нужна помощь с Redis, пинганй. Или можем в следующем PR.

issue (blocking): Не используй var

issue (blocking): Не используй var, используй const или let

var имеет function scope, что может привести к багам:

for (var i = 0; i < 3; i++) {
  setTimeout(() => console.log(i), 100);
}
// Выведет: 3, 3, 3 (ожидали: 0, 1, 2)

suggestion: Можно упростить

suggestion (non-blocking): Можно короче через деструктуризацию

Было:
const name = user.name;
const email = user.email;

Может быть:
const { name, email } = user;

issue (blocking): Не так как надо