larroy edited a comment on issue #15167: Pointwise fusion for GPU
URL: https://github.com/apache/incubator-mxnet/pull/15167#issuecomment-514808528
 
 
   Guys, let's keep the conversation constructive and focused on architecture 
and approaches rather than nits. In some cases some judicious use of "using" to 
import namespace can help. for example:
   
   Type of inode is not inmediately clear:
   ```
   const nnvm::IndexedGraph::Node& inode = idx[nid];
   ```
   
   This can be better rewritten to
   
   ```
   using ....
   [...]
   const Node& inode = idx[nid];
   ```
   Now the type is clear.
   
   Use of auto is fine here as the type is clear:
   ```
   auto node = nnvm::Node::Create();
   ```
   
   
   In this case is better to use the type and reduce long namespace resolution 
if you are constantly using classes in some namespace.
   
   This would be my preference and advice.
   Maybe we need to write a guideline approved by the community? Having these 
discussions in individual PRs slows things down too much.
   
   ❤️

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
[email protected]


With regards,
Apache Git Services

Reply via email to