Version 1 (modified by 12 years ago) (diff) | ,
---|
Introduction
The purpose of this analysis is to estimate the usefulness of the ACK/NACK system as used in OpenVPN project's development. This study extends back two years, and the data is obtained from Git logs:
$ git log --since="2 years ago"
This data is cross-referenced with the emails stored in the openvpn-devel mailing list archive. A random sampling of all the patches
The hypothesis is that the ACK/NACK process is most suitable for reviewing individual patches or small patchsets sent to the openvpn-devel mailing list by developers not intimately familiar with OpenVPN. A less bureaucratic process might be more suitable for core developers, especially when less security-sensitive parts are being modified.
Current ACK/NACK code review system
Scope
Most patches have required an ACK before going into Git "master". There are a only few exceptions:
- Patches from James Yonan's Subversion repository are merged without review
- Something else?
Evolution
The original ACK system required an ACK from one developer for the code to the Git. This process was later (add date?) split into two logical parts:
- Feature-ACK: this should be the primary consideration when reviewing patches. Merging useless patches makes no sense whatsoever.
- Code-ACK: if the feature makes sense, the next step would be to review that the code itself makes sense (e.g. has no obvious errors and follows established conventions)
Still later (add date?) a move towards lazy consensus was taken. So, as it stands now (Mar 2012), code can go to the Git repository without an ACK, but with a considerable delay.
Pros
The formal ACK/NACK system was imposed to increase OpenVPN code quality by decreasing the chances of bad code getting into the tree. The mandatory ACK essentially forces peer review. In software engineering jargon, it fulfills the roles of rationale management and static testing.
Cons
By it's nature, requiring an ACK slows down development.
Attachments (1)
-
gitlogparse.sh (1.8 KB) - added by 12 years ago.
A very ugly script used to generate statistics during ACK system review
Download all attachments as: .zip