Don't get confused. This is not about not applying the suggested changes during 
review. This is about *the lack of interaction* during review, when the 
reviewer (me) has explicitly asked for an opinion: my comment was a question, 
not a statement.

When a reviewer asks for the contributor's opinion, the expectation is to get 
an answer. Most of us use the GitHub email notifications to stay tuned to PR 
activity and use that to monitor active contributions. We also use a [code 
review dashboard](http://jclouds-pulls.herokuapp.com) to monitor the status of 
the PRs and try to minimise forgotten PRs. We could certainly do better to 
avoid situations like this, but I don't think we can safely deal with the 
uncertainty caused by the contributors silence upon a reviewer's request. 
That's my concern. If you feel like adding that to the `toString` method is not 
relevant for this PR, it is OK. It is as simple as replying to the comment! You 
did that today. It would have been easier to do it when the comment was added 
and this PR would have been merged long time ago.

There are other cases too. Take [this 
one](https://github.com/jclouds/jclouds/pull/677) as an example. What should we 
do upon the lack of feedback from the contributor? Merge it without the test 
that verifies the modified behaviour? Ping the contributor again and see if we 
can get the review conversation move forward? Do we *really* need to keep track 
of all unanswered comments and keep pinging on each? I don't think so. As 
explained in our [how to contribute 
guide](https://cwiki.apache.org/confluence/display/JCLOUDS/How+to+Contribute), 
contributors should *work with the reviewers* to get the PRs in a good state. 
And that means **interacting** when necessary.

Think about this: if a contributor asks for clarification on a reviewer's 
comment, do you find it acceptable that the reviewer just ignores that comment 
with the expectation that the contributor takes *some* action? The same 
criteria applies when the reviewer asks for the contributor's opinion.

This said, I've merged and pushed the change to master and 1.9.x, as it looks 
good to me. I was just waiting for the contributor's opinion.

Finally, I don't want to make a flame of this. We can, and will try to, do 
better monitoring PRs, but code reviews must be understood as a collaborative 
process between the contributor and the reviewer. And interaction from *both 
parts* is needed for this to work.



---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/907#issuecomment-193968570

Reply via email to