[GitHub] [incubator-mxnet] larroy edited a comment on issue #15167: Pointwise fusion for GPU

[GitBox <gi...@apache.org>]

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:
   
   ```
   const nnvm::IndexedGraph::Node& inode = idx[nid];
   ```
   
   This can be better rewritten to
   
   ```
   using ....
   [...]
   const Node& inode = idx[nid];
   ```
   
   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:
users@infra.apache.org


With regards,
Apache Git Services