-
Notifications
You must be signed in to change notification settings - Fork 0
тз 16ого спринта #3
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
Conversation
…/java-shareit into add-item-requests-and-gateway the commit.
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.
Привет, Вячеслав! Спасибо за PR.
В целом. Я добавил опциональный момент для улучшения. Но самое главное - сборка, она должны быть зеленая, иначе заругают :)
На счет покрытия вот так с ходу не скажу, локально сборки и проверки проходят, лучше сразу с вопросом к наставнику, скорее всего это уже какие то внешние проверки
| public BookingClient(@Value("${shareit-server.url}") String serverUrl, RestTemplateBuilder builder) { | ||
| super( | ||
| builder | ||
| .uriTemplateHandler(new DefaultUriBuilderFactory(serverUrl + API_PREFIX)) |
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.
Вот тут есть моментик, который можно улучшить :)
В самом клиенте происходит создание и настройка web-клинента. С архитектурной точки зрения это не очень хорошо, клиент занимается своей настройкой, это не его обязаность. Настройку лучше вынести вовне. Можно сделать конфигурацию бина и, далее, он, настроенный, будет инжектится как и остальные бины.
Единственное, аннотацию @service надо убрать, вообще, это альтернативный способ объявления бинов: мы можем на класс повесить аннотацию или создать такую вот конфигурацию если требуется провести какую то настройку при создании
Пример вот, только подправь под свою реализацию
Таким образом мы можем иметь разные настройки клиентов для разных сред: допустим на продакшн-сервере с TLS, сертификатами и пр, на тестововм стенде простое соединение, а для локального запуска мы можем, например, вообще создать заглушку, которая будет возвращать статический текст :)
EugeneLenkevich
left a comment
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.
0.8
То есть вот это помогло? Я как то постеснялся этот вариант предложить :)


No description provided.