Skip to content

Conversation

@yuriuss
Copy link

@yuriuss yuriuss commented Jan 15, 2024

The documentation contains information about the added field qrDescription, but in fact, it is not present in the repository. It has been added.

@bazavluk
Copy link

bazavluk commented Jan 16, 2024

Хорошо бы

  • поправить тесты
  • обновить readme

… it to the QR code creation test. Correcting a message in TestData.MISSING_REFUND_ID_ERROR_MESSAGE
@yuriuss
Copy link
Author

yuriuss commented Jan 16, 2024

The desired edits have been made.

@bazavluk
Copy link

Спасибо что дообогащаешь sdk, несколько моментов:

  1. Нужно еще добавить поле для QRVariable
  2. Явно задавать qrDescription через setter в QR особо нет смысла, он засеттерится через @DaTa
  3. Тест на класс QR особо не имеет смысла, если ты заполняешь его в классе, то проверять класс на наличие поля излишне, он там и так будет. Имелось в виду поправить тесты которые разваливались:)
  4. По вопросу к поддержке касательно поля, которое отвечает за наименование QR, это и есть qrDescription, поэтому нужно будет поправить формулировку в readme

@yuriuss
Copy link
Author

yuriuss commented Jan 17, 2024

Не понял насчёт 4го пункта. Нужно добавить описание того, что это поле есть и что оно хранит?

@bazavluk
Copy link

Ну вот ты отредактировал readme, но указал что это поле есть наименование платежа, по факту это описание QR, нужно поправить формулировку

@yuriuss
Copy link
Author

yuriuss commented Jun 20, 2024

@bazavluk Пофиксил все пункты

@bazavluk
Copy link

Там было изменение api в целом, проверь пож что сейчас все что в sdk все валидно, в части путей и других параметров.

  • хорошо бы добавить подписки, т.к кажется сейчас этого нет
  • сверки

@yuriuss
Copy link
Author

yuriuss commented Jun 28, 2024

Проверил, вроде всё соответствует. Добавил информацию о полях extra, subscription и обновил документацию, дополнил и подправил тесты. Номер версии надо бы поменять, т.к. много изменений. Сверку предлагаю внести в следующую версию, т.к. в этой и так достаточно изменений.

@yuriuss
Copy link
Author

yuriuss commented Jul 29, 2024

@bazavluk хотелось бы получить апрув на первую часть.

@bazavluk
Copy link

Привет!
У нас на текущий момент нет возможности прямого коммита в github, обсудим внутри как можем влить эти изменения, вернемся.

@yuriuss
Copy link
Author

yuriuss commented Jul 30, 2024

Ок. Буду ждать )

@bazavluk
Copy link

bazavluk commented Aug 5, 2024

Юра привет!
Извини за задержку, до конца недели постараемся влить, если никаких замечаний больше не будет

assertThrows(SbpException.class, () -> TestUtils.CLIENT.registerQR(badQR));
}

private boolean isValidDateFormat(String value) {
Copy link

Choose a reason for hiding this comment

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

А эта логика нужна только в рамках теста?

@yuriuss
Copy link
Author

yuriuss commented Aug 19, 2024

:((( Что-то вы забыли совсем

@bazavluk
Copy link

Юра привет!
Не забыли, посмотри плз коммент выше.
По поводу мержа - согласовываем процесс внутри, чтобы можно было мержить сразу из гитхаба

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.

3 participants