@haojin2 "a competent reviewer would be able to parse the code changes together 
with all info provided in all sections of the description of the PR" could be 
interpreted like gatekeeping. People will have a hard time moving from 
beginners to competent reviewers if each PR has only the minimum amount of 
information required by competent reviewers to understand them.

This is more a general comment than specific to this one PR, I think we should 
strive to have descriptions and explanations that can allow newbies and mxnet 
amateurs to understand more of the gist of the changes that are contributed to 
the repo. This will help to lower the barrier to entry and make the community 
more inclusive.

Each PR is an opportunity to educate and inform the community of how MXNet is 
running under the hood. Feel free to put more than less in your descriptions, 
competent reviewers will ignore the pieces of information they already know, 
and everybody else will applaud you for that :+1:

edit:
To give a concrete example, here you could explain why we sometimes offer a 
cuda and a cudnn version of the operators and how to switch between one 
implementation and the other. For this specific fix, my understanding is that 
the cuda implementation was missing a value update to `bottom_left_v` which led 
to the wrong value to be computed?

[ Full content available at: 
https://github.com/apache/incubator-mxnet/pull/12557 ]
This message was relayed via gitbox.apache.org for [email protected]

Reply via email to