Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Psalm #51

Merged
merged 2 commits into from
May 22, 2024
Merged

Psalm #51

merged 2 commits into from
May 22, 2024

Conversation

lapaliv
Copy link
Owner

@lapaliv lapaliv commented May 21, 2024

No description provided.

Copy link

@tashimotor tashimotor left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Почему решил убрать phpstan?
Они же немного по разному работают. Можно оставить оба

psalm.xml Outdated
@@ -0,0 +1,24 @@
<?xml version="1.0"?>
<psalm
errorLevel="7"

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Почему такой низкий уровень? Он тут даже не проверяет особо возвращаемые значения

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Привет. Надо же с чего-то начинать :) Планирую итерациями увеличивать

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Лучше повысь сразу уровень до нужного, добавь все ошибки что сейчас есть в baseline и потом уже рафактори. Скорее всего при итеративном подходе придется по несколько раз одно и тоже переписывать

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Хорошая идея. Спасибо

src/Features/SelectExistingRowsFeature.php Show resolved Hide resolved
@lapaliv
Copy link
Owner Author

lapaliv commented May 22, 2024

Почему решил убрать phpstan?

@tashimotor Он хуже работает с дженериками, чем psalm. У меня недавно ситуация была, когда PHPStan то видел ошибку, то не видел (разные запуски, один и тот же код). Ну и psalm на низких уровнях видит больше ошибок, чем PHPStan. Возможно дело в уровне, но мне показалось, что psalm получше справляется со своей работой, если честно :)
Весь этот ПР - исправление ошибок Psalm, которые он увидел на своем 7 уровне (де-факто на 2). PHPStan их не видел на 4 (де факто на 5м)

Они же немного по разному работают. Можно оставить оба

Принцип работы разный - это да. Но идеологически они же про одно и то же. Мне кажется, что нет смысла обе держать вместе

@lapaliv lapaliv merged commit bd1708e into master May 22, 2024
4 checks passed
@lapaliv lapaliv deleted the feat/psalm branch May 22, 2024 17:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants