Commit Digest/Guidelines: Difference between revisions
add some more review guidelines, add inclusion guidelines |
m fi typo, more specific guideline on backports |
||
Line 22: | Line 22: | ||
==New Inclusion Guidelines== | ==New Inclusion Guidelines== | ||
On average about 5% of all commits in a week will be included in the commit digest. This means that those who review the commits should choose carefully which commits should be included. To get a feeling for which messages should or should not be included, it could be useful to read existing Commit | On average about 5% of all commits in a week will be included in the commit digest. This means that those who review the commits should choose carefully which commits should be included. To get a feeling for which messages should or should not be included, it could be useful to read existing Commit Digests to see what kind of commits they contain. There is no definitive set of rules when to include a commit message and when not. However, the following gives some guidelines. | ||
Hint: you can always investigate a commit further by clicking on the commit id. This gives you helpful details about which files were modified and allows for a line by line comparison of the source code. | Hint: you can always investigate a commit further by clicking on the commit id. This gives you helpful details about which files were modified and allows for a line by line comparison of the source code. | ||
Line 32: | Line 32: | ||
Generally do not include messages if they: | Generally do not include messages if they: | ||
# are back-ports, i.e., the message basically appears two times (then do not include the backported version), | # are back-ports, i.e., the message basically appears two times (then do not include the backported/branch version, only include the version in trunk), | ||
# are refactorings, internal clean-ups, additions to the API (unless they seem to be interesting, e.g., a whole new API for Nepomuk), | # are refactorings, internal clean-ups, additions to the API (unless they seem to be interesting, e.g., a whole new API for Nepomuk), | ||
# are about CMake or desktop files, documentation changes (without bug number), | # are about CMake or desktop files, documentation changes (without bug number), |
Revision as of 13:54, 16 January 2011
Goal of this page
Composing better guidelines for reviewing and classifying commits for the Commit Digest. The source of the original inclusion guidelines can be found here.
Original Inclusion Guidelines
There is no definitive set of rules when to include a commit message and when not. However, the following gives some guidelines.
Hint: you can always investigate a commit further by clicking on the commit id. This gives you helpful details about which files were modified and allows for a line by line comparison of the source code.
In general add messages to the Commit Digest if they:
- are interesting (in general these messages have a longer text describing them),
- fix a bug, are an optimization or add a feature (for the user); this is in particular the case when there is a "broken box" symbol in the lower right corner of the message,
- new or significantly improved icons.
Generally do not include messages if they:
- are back-ports, i.e., the message basically appears two times (then do not include the backported version),
- are refactorings, internal clean-ups, additions to the API (unless they seem to be interesting, e.g., a whole new API for Nepomuk),
- are about CMake or desktop files, documentation changes (w/o bug number)
- are about unit tests ( http://en.wikipedia.org/wiki/Unit_testing )
New Inclusion Guidelines
On average about 5% of all commits in a week will be included in the commit digest. This means that those who review the commits should choose carefully which commits should be included. To get a feeling for which messages should or should not be included, it could be useful to read existing Commit Digests to see what kind of commits they contain. There is no definitive set of rules when to include a commit message and when not. However, the following gives some guidelines.
Hint: you can always investigate a commit further by clicking on the commit id. This gives you helpful details about which files were modified and allows for a line by line comparison of the source code.
In general add messages to the Commit Digest if they:
- are interesting (in general these messages have a longer text describing them),
- fix a bug, are an optimization or add a feature (for the user); this is in particular the case when there is a "broken box" symbol in the lower right corner of the message,
- new or significantly improved icons.
Generally do not include messages if they:
- are back-ports, i.e., the message basically appears two times (then do not include the backported/branch version, only include the version in trunk),
- are refactorings, internal clean-ups, additions to the API (unless they seem to be interesting, e.g., a whole new API for Nepomuk),
- are about CMake or desktop files, documentation changes (without bug number),
- are about unit tests ( http://en.wikipedia.org/wiki/Unit_testing ),
- are fixes or additions to a localization (without bug number)
- introduce trivial changes such as the resizing of a line-edit or changes in button placement.
Specific guidelines:
- Commits to the http://websvn.kde.org/trunk/kdesupport/emerge/ directory are mostly related to build fixes for KDE on Windows. In general they should not be included unless they are to be judged important based on their description.
- Often you can see commits related to accounts of developers taking place in http://websvn.kde.org/trunk/kde-common/accounts/, these commits should be excluded.
- If work on new icons is being done there can be a high frequency of commits related to them. Only include the most relevant commits stating for example that all 32x32 icons are done or if the last of the icons are committed. A maximum of three of those repeated messages are desirable for a Commit Digest. Because it can be difficult to see in advance if an icon related commit will be the last one for a specific week, the best compromise is to select the last commit in a series of commits related to icons.
Uncertain
- Commits which change the contents of the files called NEWS, AUTHORS, Changelog, README and TODO should be excluded.
- Commits which increase the version number (for alpha and beta versions) should be excluded, unless it's for a new release (or not for new releases either? comment please?).
- What do we with reverts of commits? For example removal of code (is that considered refactoring which should be removed according to guidelines), or more specific commits which remove a feature because the feature is not considered ready for the next stable release?
- What do we do with the commits which often occur since the migration to Git which mention a branch merge?
Classification Guidelines
Bug Fixes: this category contains all commits which mention a bug number in the message. It also contains commits which fix something according to the comment but don't have a bug report attached (these commits need to be important though, remember the 5% rule!).
Optimize: this category is only meant for commits that make things either faster, use less memory, or in rare cases, make the interface more streamlined ("now this can be done in 2 clicks instead of 10").
Other: this category is for commits that don't really fit anywhere else: for example for commits that are really interesting or funny even if they otherwise wouldn't be important (like someone writing in the message that they learned something really important, etc.).
Specific guidelines:
New icons and other new artwork should be classified under the area 'User Interface' and under the type 'Feature'.