wiki:ACKSystemReview

Version 1 (modified by Samuli Seppänen, 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:

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)

Download all attachments as: .zip