fix: resolve build issues and update gitignore#11
Conversation
- fixes infrastructure/dependency issue
null8626
left a comment
null8626
left a comment
There was a problem hiding this comment.
Hai! There are sadly still many issues I have with this pull request. Most of which are docstrings related, others are code that still needs more polishing or consistency. Thanks buffer!
|
|
||
| set(CMAKE_BUILD_TYPE Debug CACHE STRING "Build type") | ||
| option(BUILD_SHARED_LIBS "Build shared libraries" ON) | ||
| option(ENABLE_CORO "Support for C++20 coroutines" OFF) |
There was a problem hiding this comment.
Why did you remove ENABLE_CORO?
There was a problem hiding this comment.
This option lets users of this library choose whether to utilize C++20 coroutines or not (done through callbacks, but compatible with C++17).
| @@ -1,15 +1,41 @@ | |||
| if(WIN32 AND NOT EXISTS ${CMAKE_SOURCE_DIR}/deps/dpp.lib) | |||
| string(TOLOWER ${CMAKE_BUILD_TYPE} INSTALL_DPP_BUILD_TYPE) | |||
| execute_process(COMMAND powershell "-NoLogo" "-NoProfile" "-File" ".\\install_dpp_msvc.ps1" ${INSTALL_DPP_BUILD_TYPE} WORKING_DIRECTORY ${CMAKE_SOURCE_DIR}) | |||
There was a problem hiding this comment.
The install_dpp_msvc.ps1 script here automatically installs the D++ library for Microsoft Visual C++ users. Why remove it?
There was a problem hiding this comment.
PS: I removed every DPP-related installation scripts because I cannot and don't know how to build it using that on Windows. It shows too much build errors when I first ran it, I don't know if I did something wrong or something. That was the first time I cloned the repository btw
There was a problem hiding this comment.
Can you show me what the errors are?
| #include <vector> | ||
| #include <string> | ||
| #include <map> | ||
| #include "topgg/export.h" |
There was a problem hiding this comment.
I feel like this should've been #include <topgg/export.h>. Not that big of an issue, but this would've made it consistent!
| virtual ~account() = default; | ||
|
|
||
| /** | ||
| * @brief The account's Discord ID. |
There was a problem hiding this comment.
Why remove the docstrings of this class' properties?
| ~bot() override = default; | ||
|
|
||
| /** | ||
| * @brief The Discord bot's discriminator. |
There was a problem hiding this comment.
Why remove the docstrings of this class' properties?
|
|
||
| #ifdef DPP_CORO | ||
| /** | ||
| * @brief An async result class that gets returned from every C++20 coroutine HTTP response. |
There was a problem hiding this comment.
Why remove the docstrings of this class' properties?
| * @brief Data received from a bot vote webhook | ||
| */ | ||
| struct bot_vote_data { | ||
| dpp::snowflake user_id; ///< ID of the user who voted |
There was a problem hiding this comment.
These docstrings should've used the proper format consistent with the other classes in this library (e.g: use @brief, @version, etc).
| bool is_weekend; ///< Whether the vote was done during weekend | ||
| std::string query; ///< Query string data if any | ||
|
|
||
| static bot_vote_data from_json(const nlohmann::json& j) { |
There was a problem hiding this comment.
This function body should be moved to webhook.cpp as it does not depend on any generics.
| : cluster_(cluster), port_(port), auth_token_(auth_token) { | ||
|
|
||
| // Set up interaction handler for webhooks | ||
| cluster_.on_interaction_create([this](const dpp::interaction_create_t& event) { |
There was a problem hiding this comment.
This function body should be moved to webhook.cpp as it does not depend on any generics.
|
|
||
| static bot_vote_data from_json(const nlohmann::json& j) { | ||
| bot_vote_data data; | ||
| data.user_id = dpp::snowflake(j["user"].get<std::string>()); |
There was a problem hiding this comment.
Try using those DESERIALIZE() macros!
ghost
closed this by deleting the head repository
2 participants
WIP
What's new in this branch?:
Webhookfeature