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] service renaming and comments. #1953

Merged
merged 1 commit into from
Jan 27, 2025
Merged

Conversation

KerstinKeller
Copy link
Contributor

Description

A little bit of type renaming, and some comments added.

@KerstinKeller KerstinKeller marked this pull request as ready for review January 27, 2025 15:34
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

switch (service_response_.call_state)
{
// service successful executed
case eCAL::eCallState::executed:
{
std::cout << "Received response for method " << service_response_.service_method_id.method_name << " : " << service_response_.response << " from service id " << entity_id_.entity_id << " from host " << service_response_.service_method_id.service_id.host_name << std::endl;
std::cout << "Received response for method " << service_response_.service_method_information.method_name << " : " << service_response_.response << " from service id " << service_response_.service_method_id.service_id.entity_id << " from host " << service_response_.service_method_id.service_id.host_name << std::endl;
Copy link
Contributor

Choose a reason for hiding this comment

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

warning: do not use 'std::endl' with streams; use '\n' instead [performance-avoid-endl]

Suggested change
std::cout << "Received response for method " << service_response_.service_method_information.method_name << " : " << service_response_.response << " from service id " << service_response_.service_method_id.service_id.entity_id << " from host " << service_response_.service_method_id.service_id.host_name << std::endl;
std::cout << "Received response for method " << service_response_.service_method_information.method_name << " : " << service_response_.response << " from service id " << service_response_.service_method_id.service_id.entity_id << " from host " << service_response_.service_method_id.service_id.host_name << '\n';

}
break;
// service execution failed
case eCAL::eCallState::failed:
std::cout << "Received error for method " << service_response_.service_method_id.method_name << " : " << service_response_.error_msg << " from service id " << entity_id_.entity_id << " from host " << service_response_.service_method_id.service_id.host_name << std::endl;
std::cout << "Received error for method " << service_response_.service_method_information.method_name << " : " << service_response_.error_msg << " from service id " << service_response_.service_method_id.service_id.entity_id << " from host " << service_response_.service_method_id.service_id.host_name << std::endl;
Copy link
Contributor

Choose a reason for hiding this comment

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

warning: do not use 'std::endl' with streams; use '\n' instead [performance-avoid-endl]

Suggested change
std::cout << "Received error for method " << service_response_.service_method_information.method_name << " : " << service_response_.error_msg << " from service id " << service_response_.service_method_id.service_id.entity_id << " from host " << service_response_.service_method_id.service_id.host_name << std::endl;
std::cout << "Received error for method " << service_response_.service_method_information.method_name << " : " << service_response_.error_msg << " from service id " << service_response_.service_method_id.service_id.entity_id << " from host " << service_response_.service_method_id.service_id.host_name << '\n';

@hannemn hannemn added the cherry-pick-to-NONE Don't cherry-pick these changes label Jan 27, 2025
Copy link
Contributor

@hannemn hannemn left a comment

Choose a reason for hiding this comment

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

LGTM

@KerstinKeller KerstinKeller merged commit e9defe3 into master Jan 27, 2025
22 of 23 checks passed
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