On 02/10/15 06:13, Matthew Ahrens wrote:
This is a reminder to review the (much anticipated) resumable send/receive code, or let me know if you need more time, by tomorrow.
This patch contains in addition to the feature implementation many small style changes and preparatory commits. While those changes are for the better and it's good to have them in, I feel they are an unnecessary burden to the reviewer. In the past I have made good experiences with splitting up larger patches. I think there are at least 5 or 6 patches that could be extracted and stand for themselves, all very easy to review. As a reviewer, you can walk through them one by one, leaving only the essence of the feature in the last one. This way, it's much easier to concentrate on it. It could probably cut up to 30% off the main patch. Also, style changes can introduce errors, and having them in small standalone patches increases the visibility and they will probably get more reviews (everyone loves small reviews). For the commit to the repository the commits could also get squashed again, if that fits the current integration workflow better. Unfortunately ReviewBoard doesn't seem to have support for multiple commits in a single review request. This is not to be seen as a request to change this review request, I already worked through it. But maybe we can adapt the process for coming reviews a bit, to make it easier for potential reviewers. Thanks, Arne _______________________________________________ developer mailing list [email protected] http://lists.open-zfs.org/mailman/listinfo/developer
