-
Notifications
You must be signed in to change notification settings - Fork 28
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
Dev 770 #5
Dev 770 #5
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Вы так и не добавили ни одного теста PHPUnit, поэтому нет никакой необходимости в пакете phpunit/phpunit.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Предлагаю:
- Изменить название метода на
getContactsByPhone($phone, array $select=[])
. У телефона нет никаких иных свойств кроме номера. - Избавиться от дублирования кода и вызывать внутри этого метода уже существующий метод
getContactList()
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Предлагаю:
- Заменить метод
getTasksList()
наgеtTaskList(array $filter = [], array $select = [], array $order = []): Generator
с именем, сигнатурой и реализацией полностью аналогичной уже существующим методам для других сущностей. - Заменить метод
getTasks()
наgetTasksById($taskIds)
, возвращающий массив задач. Внутри него вызывать методgеtTaskList()
. Функцияarray_chunk()
не требуется, так как методgetList()
и так возвращает не более 50 записей в ответе.
Предлагаю привести вновь разработанный код из pull request в соответствие стандарту PSR-12: Extended Coding Style, как это было сделано начиная с v1.5.0 данной библиотеки. |
В отсутствии, в разумные сроки, обратной связи закрываю данный pull request. |
метод для получения контактов по номеру телефона