Skip to content
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

[core] remove namespace v6, keep v5. Remove build of compatibility ta… #1957

Merged
merged 3 commits into from
Jan 28, 2025

Conversation

KerstinKeller
Copy link
Contributor

…rget.

Description

Related issues

Cherry-pick to

Copy link
Contributor

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

clang-tidy made some suggestions

@@ -401,7 +401,7 @@ namespace eCAL
return(true);
}

bool CPublisherImpl::SetEventCallback(const v6::PubEventCallbackT callback_)
bool CPublisherImpl::SetEventCallback(const PubEventCallbackT callback_)
Copy link
Contributor

Choose a reason for hiding this comment

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

warning: the const qualified parameter 'callback_' is copied for each invocation; consider making it a reference [performance-unnecessary-value-param]

ecal/core/src/pubsub/ecal_publisher_impl.h:88:

-     bool SetEventCallback(const PubEventCallbackT callback_);
+     bool SetEventCallback(const PubEventCallbackT& callback_);
Suggested change
bool CPublisherImpl::SetEventCallback(const PubEventCallbackT callback_)
bool CPublisherImpl::SetEventCallback(const PubEventCallbackT& callback_)

@@ -233,7 +233,7 @@ namespace eCAL
return(true);
}

bool CSubscriberImpl::SetEventIDCallback(const v6::SubEventCallbackT callback_)
bool CSubscriberImpl::SetEventIDCallback(const SubEventCallbackT callback_)
Copy link
Contributor

Choose a reason for hiding this comment

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

warning: the const qualified parameter 'callback_' is copied for each invocation; consider making it a reference [performance-unnecessary-value-param]

Suggested change
bool CSubscriberImpl::SetEventIDCallback(const SubEventCallbackT callback_)
bool CSubscriberImpl::SetEventIDCallback(const SubEventCallbackT& callback_)

ecal/core/src/pubsub/ecal_subscriber_impl.h:79:

-     bool SetEventIDCallback(const SubEventCallbackT callback_);
+     bool SetEventIDCallback(const SubEventCallbackT& callback_);

@@ -95,7 +95,7 @@ namespace eCAL
Logging::Log(Logging::log_level_debug1, "v5::CServiceClientImpl: Creating service client with name: " + service_name_);

// Define the event callback to pass to CServiceClient
v6::ClientEventCallbackT event_callback = [this](const SServiceId& service_id_, const v6::SClientEventCallbackData& data_)
eCAL::ClientEventCallbackT event_callback = [this](const SServiceId& service_id_, const eCAL::SClientEventCallbackData& data_)
Copy link
Contributor

Choose a reason for hiding this comment

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

warning: variable 'event_callback' of type 'eCAL::ClientEventCallbackT' (aka 'function<void (const SServiceId &, const SClientEventCallbackData &)>') can be declared 'const' [misc-const-correctness]

Suggested change
eCAL::ClientEventCallbackT event_callback = [this](const SServiceId& service_id_, const eCAL::SClientEventCallbackData& data_)
eCAL::ClientEventCallbackT const event_callback = [this](const SServiceId& service_id_, const eCAL::SClientEventCallbackData& data_)

@@ -57,7 +57,7 @@ namespace eCAL
}

// Define the event callback to pass to CServiceClient
v6::ServerEventCallbackT event_callback = [this](const SServiceId& service_id_, const v6::SServerEventCallbackData& data_)
eCAL::ServerEventCallbackT event_callback = [this](const SServiceId& service_id_, const eCAL::SServerEventCallbackData& data_)
Copy link
Contributor

Choose a reason for hiding this comment

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

warning: variable 'event_callback' of type 'eCAL::ServerEventCallbackT' (aka 'function<void (const SServiceId &, const struct SServerEventCallbackData &)>') can be declared 'const' [misc-const-correctness]

Suggested change
eCAL::ServerEventCallbackT event_callback = [this](const SServiceId& service_id_, const eCAL::SServerEventCallbackData& data_)
eCAL::ServerEventCallbackT const event_callback = [this](const SServiceId& service_id_, const eCAL::SServerEventCallbackData& data_)

@KerstinKeller KerstinKeller added the cherry-pick-to-NONE Don't cherry-pick these changes label Jan 28, 2025
@KerstinKeller KerstinKeller marked this pull request as ready for review January 28, 2025 12:04
Copy link
Contributor

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

clang-tidy made some suggestions

/**
* @brief Service client wrapper class.
**/
class ECAL_API_CLASS CServiceClient
Copy link
Contributor

Choose a reason for hiding this comment

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

warning: variable 'CServiceClient' is non-const and globally accessible, consider making it const [cppcoreguidelines-avoid-non-const-global-variables]

  class ECAL_API_CLASS CServiceClient
                       ^

/**
* @brief Service Server wrapper class.
**/
class ECAL_API_CLASS CServiceServer
Copy link
Contributor

Choose a reason for hiding this comment

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

warning: variable 'CServiceServer' is non-const and globally accessible, consider making it const [cppcoreguidelines-avoid-non-const-global-variables]

  class ECAL_API_CLASS CServiceServer
                       ^

Copy link
Contributor

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

clang-tidy made some suggestions

ECAL_CORE_NAMESPACE_V6
{
CServiceClient::CServiceClient(const std::string & service_name_, const ServiceMethodInfoSetT& method_information_set_, const ClientEventCallbackT event_callback_)
CServiceClient::CServiceClient(const std::string & service_name_, const ServiceMethodInformationSetT& method_information_set_, const ClientEventCallbackT event_callback_)
Copy link
Contributor

Choose a reason for hiding this comment

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

warning: the const qualified parameter 'event_callback_' is copied for each invocation; consider making it a reference [performance-unnecessary-value-param]

ecal/core/include/ecal/service/client.h:55:

-       CServiceClient(const std::string& service_name_, const ServiceMethodInformationSetT& method_information_set_ = ServiceMethodInformationSetT(), const ClientEventCallbackT event_callback_ = ClientEventCallbackT());
+       CServiceClient(const std::string& service_name_, const ServiceMethodInformationSetT& method_information_set_ = ServiceMethodInformationSetT(), const ClientEventCallbackT& event_callback_ = ClientEventCallbackT());
Suggested change
CServiceClient::CServiceClient(const std::string & service_name_, const ServiceMethodInformationSetT& method_information_set_, const ClientEventCallbackT event_callback_)
CServiceClient::CServiceClient(const std::string & service_name_, const ServiceMethodInformationSetT& method_information_set_, const ClientEventCallbackT& event_callback_)


private:
// Private constructor to enforce creation through factory method
CServiceClientImpl(const std::string& service_name_, const ServiceMethodInfoSetT& method_information_map_, const ClientEventCallbackT& event_callback_);
CServiceClientImpl(const std::string& service_name_, const ServiceMethodInformationSetT& method_information_map_, const ClientEventCallbackT& event_callback_);
Copy link
Contributor

Choose a reason for hiding this comment

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

warning: function 'eCAL::CServiceClientImpl::CServiceClientImpl' has a definition with different parameter names [readability-inconsistent-declaration-parameter-name]

      CServiceClientImpl(const std::string& service_name_, const ServiceMethodInformationSetT& method_information_map_, const ClientEventCallbackT& event_callback_);
      ^
Additional context

ecal/core/src/service/ecal_service_client_impl.cpp:86: the definition seen here

  CServiceClientImpl::CServiceClientImpl(
                      ^

ecal/core/src/service/ecal_service_client_impl.h:53: differing parameters are named here: ('method_information_map_'), in definition: ('method_information_set_')

      CServiceClientImpl(const std::string& service_name_, const ServiceMethodInformationSetT& method_information_map_, const ClientEventCallbackT& event_callback_);
      ^

@KerstinKeller KerstinKeller merged commit ccab005 into master Jan 28, 2025
21 checks passed
@KerstinKeller KerstinKeller deleted the feature/remove-v6-namespace branch February 20, 2025 13:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cherry-pick-to-NONE Don't cherry-pick these changes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants