Skip to content

Conversation

MaximilianToe
Copy link
Owner

No description provided.

Copy link

Code coverage report is ready! 📈

Copy link

Code coverage report is ready! 📈

Copy link

Code coverage report is ready! 📈

Copy link

Code coverage report is ready! 📈

Copy link

Code coverage report is ready! 📈

Copy link

Code coverage report is ready! 📈

Copy link

@lammjo lammjo left a comment

Choose a reason for hiding this comment

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

Only a very superficial review, but please go ahead and create a PR to upstream if you feel comfortable.

CMakeLists.txt Outdated
Comment on lines 49 to 55
file(GLOB_RECURSE SRC_FILES "${CMAKE_CURRENT_SOURCE_DIR}/src/*.cpp")

add_library(${PROJECT_NAME} ${SRC_FILES})
add_library(${PROJECT_NAME} ${SRC_FILES}
src/client/usubscription/v3/RpcClientUSubscription.cpp
include/up-cpp/client/usubscription/v3/RpcClientUSubscription.h
include/up-cpp/client/usubscription/v3/USubscription.h
)
Copy link

Choose a reason for hiding this comment

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

This construction seems odd. Why include only these particular headers and not others? Why include the headers at all? Shouldn't the source file already be picked up by the glob above?

///
/// @param topic The `v1::UUri` representing the topic to fetch.
///
/// @return An `FetchSubscriptionsRequest` configured for the specified
Copy link

Choose a reason for hiding this comment

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

Suggested change
/// @return An `FetchSubscriptionsRequest` configured for the specified
/// @return A `FetchSubscriptionsRequest` configured for the specified

static FetchSubscriptionsRequest buildFetchSubscriptionsRequest(
const v1::UUri& topic);

/// @brief Build fetch subscritions request for a given subscriber.
Copy link

Choose a reason for hiding this comment

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

Suggested change
/// @brief Build fetch subscritions request for a given subscriber.
/// @brief Build fetch subscriptions request for a given subscriber.

/// @return An `UnsubscribeRequest` configured for the specified topic.
static UnsubscribeRequest buildUnsubscribeRequest(const v1::UUri& topic);

/// @brief Build fetch subscritions request for a given topic.
Copy link

Choose a reason for hiding this comment

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

Suggested change
/// @brief Build fetch subscritions request for a given topic.
/// @brief Build fetch subscriptions request for a given topic.

/// @param subscriber The `SubscriberInfo` representing the subscriber to
/// fetch.
///
/// @return An `FetchSubscriptionsRequest` configured for the specified
Copy link

Choose a reason for hiding this comment

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

Suggested change
/// @return An `FetchSubscriptionsRequest` configured for the specified
/// @return A `FetchSubscriptionsRequest` configured for the specified

///
/// @param topic The `v1::UUri` representing the topic to fetch.
///
/// @return An `FetchSubscribersRequest` configured for the specified topic.
Copy link

Choose a reason for hiding this comment

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

Suggested change
/// @return An `FetchSubscribersRequest` configured for the specified topic.
/// @return A `FetchSubscribersRequest` configured for the specified topic.

/// @param topic The `v1::UUri` representing the topic to (un)register
/// for/from.
///
/// @return An `NotificationsRequest` configured for the specified topic.
Copy link

Choose a reason for hiding this comment

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

Suggested change
/// @return An `NotificationsRequest` configured for the specified topic.
/// @return A `NotificationsRequest` configured for the specified topic.


} // namespace uprotocol::core::usubscription::v3

#endif // UP_CPP_CLIENT_USUBSCRIPTION_V3_RPCCLIENTUSUBSCRIPTION_H
Copy link

Choose a reason for hiding this comment

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

Please add newline

return uri;
}

} // namespace uprotocol::core::usubscription::v3
Copy link

Choose a reason for hiding this comment

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

Add newline please

Copy link

github-actions bot commented Jun 7, 2025

Code coverage report is ready! 📈

Copy link

github-actions bot commented Jun 7, 2025

Code coverage report is ready! 📈

@MaximilianToe MaximilianToe force-pushed the RpcClientUSubscription branch from 7801d70 to 796a5b1 Compare June 7, 2025 15:03
Copy link

github-actions bot commented Jun 7, 2025

Code coverage report is ready! 📈

Copy link

github-actions bot commented Jun 7, 2025

Code coverage report is ready! 📈

Copy link

github-actions bot commented Jun 7, 2025

Code coverage report is ready! 📈

@MaximilianToe MaximilianToe force-pushed the RpcClientUSubscription branch from d80a943 to fb81873 Compare June 7, 2025 15:13
Copy link

github-actions bot commented Jun 7, 2025

Code coverage report is ready! 📈

Copy link

github-actions bot commented Jun 9, 2025

Code coverage report is ready! 📈

@MaximilianToe MaximilianToe force-pushed the RpcClientUSubscription branch from ede937e to bf9d68e Compare June 9, 2025 09:19
Copy link

github-actions bot commented Jun 9, 2025

Code coverage report is ready! 📈

@MaximilianToe MaximilianToe force-pushed the RpcClientUSubscription branch from bf9d68e to 563924e Compare June 9, 2025 09:31
Copy link

github-actions bot commented Jun 9, 2025

Code coverage report is ready! 📈

@MaximilianToe MaximilianToe force-pushed the RpcClientUSubscription branch from 563924e to 798696b Compare June 9, 2025 09:33
Copy link

github-actions bot commented Jun 9, 2025

Code coverage report is ready! 📈

@MaximilianToe MaximilianToe force-pushed the RpcClientUSubscription branch from 798696b to 303636e Compare June 9, 2025 09:51
Copy link

github-actions bot commented Jun 9, 2025

Code coverage report is ready! 📈

@MaximilianToe MaximilianToe force-pushed the RpcClientUSubscription branch from 303636e to 47973ea Compare June 9, 2025 10:34
Copy link

github-actions bot commented Jun 9, 2025

Code coverage report is ready! 📈

Copy link

Code coverage report is ready! 📈

Copy link

Code coverage report is ready! 📈

@MaximilianToe MaximilianToe force-pushed the RpcClientUSubscription branch from 701bfdf to 6aae8ea Compare August 11, 2025 19:45
Copy link

Code coverage report is ready! 📈

Copy link

Code coverage report is ready! 📈

@MaximilianToe MaximilianToe force-pushed the RpcClientUSubscription branch from 2d22bb9 to 3b8a7ca Compare August 24, 2025 16:37
Copy link

Code coverage report is ready! 📈

@MaximilianToe MaximilianToe force-pushed the RpcClientUSubscription branch from 3b8a7ca to 4375d15 Compare August 24, 2025 16:45
Copy link

Code coverage report is ready! 📈

Copy link

Code coverage report is ready! 📈

@MaximilianToe MaximilianToe force-pushed the RpcClientUSubscription branch from ccb349b to 15e8bcb Compare August 24, 2025 17:44
Copy link

Code coverage report is ready! 📈

@MaximilianToe MaximilianToe force-pushed the RpcClientUSubscription branch from 15e8bcb to 0c41572 Compare August 24, 2025 17:59
Copy link

Code coverage report is ready! 📈

@MaximilianToe MaximilianToe force-pushed the RpcClientUSubscription branch from 0c41572 to c8fdce4 Compare August 24, 2025 19:01
Copy link

Code coverage report is ready! 📈

@MaximilianToe MaximilianToe force-pushed the RpcClientUSubscription branch from c8fdce4 to f899ee8 Compare August 24, 2025 19:05
Copy link

Code coverage report is ready! 📈

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.

2 participants