Skip to content

Conversation

@haufont
Copy link

@haufont haufont commented Oct 5, 2023

Resolves #227

@github-actions
Copy link

github-actions bot commented Oct 5, 2023

CLA Assistant Lite bot All contributors have signed the CLA ✍️ ✅

@haufont
Copy link
Author

haufont commented Oct 5, 2023

I hereby agree to the terms of the CLA available at: https://yandex.ru/legal/cla/

Copy link
Contributor

@theg4sh theg4sh left a comment

Choose a reason for hiding this comment

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

Greet job, but have few comments.

add_subdirectory(mysql "${CMAKE_BINARY_DIR}/userver/mysql")
endif()

if (USERVER_FEATURE_CORE)
Copy link
Contributor

Choose a reason for hiding this comment

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

Looks like this feature should be named as USERVER_FEATURE_TELEGRAM.

@@ -0,0 +1,12 @@
--------------------------------------------------------------------------------
Copy link
Contributor

Choose a reason for hiding this comment

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

Better to describe api in openapi schema format. This is almost a standard to communicate between services and many code generator tools is supported it.

Have you seen json schema? https://core.telegram.org/schema/json
More details available here: https://core.telegram.org/schema

Anyway, looks like all of request-response could be generated in-fly in build directory.

Copy link
Contributor

Choose a reason for hiding this comment

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

wrong place, should be in ../client/ folder


virtual LogOutRequest LogOut(const RequestOptions& request_options) = 0;

virtual SendAnimationRequest SendAnimation(
Copy link
Contributor

Choose a reason for hiding this comment

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

Looks like the most of this method does not required at all.
They do the same as public FormRequest<T>(T::Parameters{...}, request_options);


private:
template <typename Request>
Request FormRequest(
Copy link
Contributor

Choose a reason for hiding this comment

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

FormRequest -> CreateRequest
and make it public. You'll got the same result, but less code.

const components::ComponentConfig& config,
const components::ComponentContext& context);

telegram::bot::RequestOptions request_options_;
Copy link
Contributor

Choose a reason for hiding this comment

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

This field could be moved inside of components clients and provided within client.hpp.
It looks statically configurable and may be provided by ComponentConfig or dynamic_config

};

template <typename method>
class [[nodiscard]] Request {
Copy link
Contributor

Choose a reason for hiding this comment

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

It looks like a real client class client/client.hpp, as it is actually should be.
Anyway, telegram/bot/client is a better place for it.

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.

Implement sample for the Telegram chat bot

2 participants