My personal thoughts with regards to MoveIt contributors don't review other PRs:
It took me a long time to feel like I understood MoveIt enough to contribute, and it definitely takes longer to be able to understand someone else's contribution, especially if it's in a part of the code that you aren't familiar with. MoveIt's a very big project, so this happens pretty frequently: personally, I don't feel like I have enough knowledge to review anything outside of MoveIt Core and the OMPL interface, and don't do so often.
However, I think the biggest problem is that it's not clear how to review a PR when you *aren't a maintainer*. While some comments in reviews are algorithmic or performance based, I think you could say that 30-50% of a code review is subjective, given that there are a lot of equally valid ways to do the same thing. Personally, I'd rather not review a PR with some advice, only for that advice to contradict the advice of the maintainers (or anyone else with more experience than me) when they review it and end up confusing first time contributors.
In general, It's not clear how to help the maintainers the best when reviewing. Maybe there should be a section in CONTRIBUTING about reviewing other PRs, what to focus on, what to leave to the maintainers for codebase consistency, etc.
---
[Visit Topic](
https://discourse.ros.org/t/ros-quality-assurance-working-group-december-2018-meeting-notes/7045/2) or reply to this email to respond.
If you do not want to receive messages from ros-users please use the unsubscribe link below. If you use the one above, you will stop all of ros-users from receiving updates.
______________________________________________________________________________
ros-users mailing list
ros-users@lists.ros.org
http://lists.ros.org/mailman/listinfo/ros-users
Unsubscribe: <
http://lists.ros.org/mailman//options/ros-users>
(text/plain)