[GitHub] [trafficserver] brbzull0 commented on a change in pull request #8689: Hoststatus remove stats creation.

Wed, 16 Mar 2022 04:40:33 -0700 


brbzull0 commented on a change in pull request #8689:
URL: https://github.com/apache/trafficserver/pull/8689#discussion_r827912983



##########
File path: src/traffic_server/HostStatus.cc
##########
@@ -44,8 +44,8 @@ struct HostCmdInfo {
 
 } // namespace
 
-ts::Rv<YAML::Node> server_get_status(std::string_view const &id, YAML::Node 
const &params);
-ts::Rv<YAML::Node> server_set_status(std::string_view const &id, YAML::Node 
const &params);
+ts::Rv<YAML::Node> server_get_status(std::string_view const id, YAML::Node 
const &params);
+ts::Rv<YAML::Node> server_set_status(std::string_view const id, YAML::Node 
const &params);

Review comment:
       > It's redundant to use 'string_view const' instead of just plain 
'string_view' but harmless.
   
   Totally, but the reason is this:
   
   
https://github.com/apache/trafficserver/blob/68e69e27b87576abfaa7a1bbeb8394795c522e82/mgmt2/rpc/jsonrpc/JsonRPCManager.h#L64
   
   It is expected to be like this.
   
   --
   The reason is `const&` is becase I wanted  to make it clear that this should 
not mutate despite the fact that you cannot change it if it was only a 
`std::string_view`. I always thought that this is ambiguous and if someone try 
to change it well, good luck. Now kind of I am inclined to remove the `const &`
   




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to