Chazovs / retailcrm-validator

MIT License
0 stars 1 forks source link

Review #2

Closed Neur0toxine closed 3 years ago

Neur0toxine commented 3 years ago

https://github.com/Chazovs/retailcrm-validator/blob/main/lib/RetailCrm/Validator/CrmUrl.php#L17 - здесь тексты ошибок на английском должны быть. Аналогично для комментариев и всего остального.

https://github.com/Chazovs/retailcrm-validator/blob/main/lib/RetailCrm/Validator/CrmUrlValidator.php#L42 - здесь не все части URL проверяются, которые нужно проверять.

https://github.com/Chazovs/retailcrm-validator/blob/main/lib/RetailCrm/Validator/CrmUrlValidator.php#L75 - здесь можно получить warning если file_get_contents не отработал и вернулся false.

https://github.com/Chazovs/retailcrm-validator/blob/main/lib/RetailCrm/Validator/CrmUrlValidator.php#L28 - не учитывается, что host может отсутствовать в результате.

https://github.com/Chazovs/retailcrm-validator/blob/main/lib/RetailCrm/Validator/CrmUrlValidator.php#L25 - не учитывается тот факт, что parse_url будет пытаться распарсить даже невалидные ссылки. Нужно проверять корректность URL. Например, с помощью filter_var и FILTER_VALIDATE_URL.

https://github.com/Chazovs/retailcrm-validator/blob/main/tests/bootstrap.php#L3 - вряд ли этот workaround здесь нужен.

https://github.com/Chazovs/retailcrm-validator/blob/main/composer.json#L19 - symfony/validator должен быть в основных, а не в dev-зависимостях.

https://github.com/Chazovs/retailcrm-validator/blob/main/composer.json#L5 - RetailCRM с маленькой буквы написано.

Также в проекте не настроены phpcs, phpmd и phpstan. Их необязательно даже с нуля настраивать - всё необходимое можно взять с API-клиента, например.

Как и в случае с гошным валидатором, логика слишком сложна здесь. Предлагаю использовать предложенную в ревью гошного валидатора логику.

Chazovs commented 3 years ago

https://github.com/Chazovs/retailcrm-validator/commit/763b2a2337624d1b3e4c126409fe3d1ae5864f81