Subversion: checklist for correct commits
We assume that the reader: a) works in a team; and b) realized the need to work properly with version control systems, or at least be faced with the need to use one.
The examples will use Subversion, although all of the recommendations are fully applicable to any other version control system.
Roughly divide the project development phases into three - a debut, middle game and endgame.
')
In the debut of the new code is written in huge chunks, often whole chunks of the system are transferred from place to place. The release is far, there are no special requirements for the general state of the system. It is even possible to break the system down to some limits.
In the middlegame, the system as a whole has stabilized, the product is nearing release. Refactoring is clearer, although sometimes quite extensive. At this stage, it is already expected that the system as a whole works - at least, a broken repository is reproached.
Finally, the system enters the endgame just before the release and immediately after. For web applications, new relatively small features are constantly being added, and major changes are first tested on branches. For more traditional applications, on the contrary, a branch is created for maintenance releases, and the development of the next large version continues on the trunk.
In the debut, commits are made relatively rarely. For example, creating a new class, a new relationship, or writing a preliminary version of a fairly large piece of code can be a good commit point. For some programmers, especially high-performance ones, there may even be a convenient commit tied to time — two or three times a day. The repository can be in a fairly “parsed” state, to the extent that the extracted code is simply not compiled.
In the middle game, the stream of commits is much more structured. Commits more often, and are tied only to structural changes. Most commits are no larger than average. Good points for a commit are: closing a task or a bug in the tracker, refactoring, adding or fixing another feature. The repository is almost always in a more or less efficient state, and broken repositories are reproached (and the fact of breaking causes a karmic blow).
Finally, in the endgame, commits are somehow controlled rather tightly. In general, the very fact of working on a project in this phase should have some justification - usually this is a bug or an important feature postponed after the release. The commits on this phase quickly go to production, so they: are clearly tied to the bug tracker; relatively small; well documented; well tested and thought out - introducing additional bugs at this stage causes a serious karmic blow).
Based on many years of experience in developing, supporting and reviewing fairly large software systems, we have compiled a set of important recommendations. They are designed, firstly, to increase the depth of awareness when working on a project; secondly, to facilitate the work of the code-reviewer - and therefore the likelihood of finding a bug or a flaw; thirdly, to alleviate the fate of a person who will be forced to understand the history of a project sometime in the future (often the author of current commits himself will be such a person).
In general, in our opinion, the depth of understanding by the programmer of the project he is working on strongly correlates with the quality of the commits that he does in the process.
The commits should be logical. A commit must correspond to one structural unit — a new file, a new block, a new employee, a new class, a new relation, a new feature, a closed bug, a single refactoring, a chapter of documentation, a correction of documentation, etc.
The meaning of this requirement is to play for the good of the future. By observing it, you can be sure that a particular commit in a stream is either safe (for example, fix documentation), or it can be freely rolled out with confidence that exactly one logical change has been rolled back. This is an invaluable property for binary search errors. Moreover, it is obvious that logical changes are much easier to see in the code review process.
The commits should be tested (with the exception of the debut phase). Ideally, the change is generally tested by unit tests. In the extreme case, a commit must be checked at least once - even the most innocuous corrections can lead to disastrous consequences for karma. Of course, a commit should be syntactically valid - fix to fixes a blow to the image.
Commits should be well documented. We are silent about empty journal messages - this is generally the worst that can be. This is permissible only for employees who are completely distant from programming (such as designers) who hardly agreed to your subversation at all. And it makes sense to work on the cultural level, even such people! A commit with the description of “Fix” or “Fixed” is allowed only if there are no more than two or three lines in the differential. If more - you need to write in more detail.
Separately, it is necessary to consider the issue of a journal message consisting of several unrelated items. It is clear that such a message is a symptom of a true problem — one that your commit consists of several unrelated parts. The best solution in this case would be to cancel the attempt of the commit and try to split it into several parts.
In the endgame phase, poorly documented commits should be strictly censured. Also, if a commit is a closure of a bug, then the bug number should be mentioned in a journal message. The same Trac integrates with Subversion, identifying ticket numbers in journal messages and automatically turning them into appropriate links.
Before committing, you need to view diffs, as well as keep track of new files that have not been added to the repository, and add them. If you use TortoiseSVN, then it has a special menu item called “Check for modifications”. The command-line client for this purpose are the svn st commands (there are no dangling files) and svn diff | more (for reviewing changes before a commit).
This rule must be observed in order not to accidentally commit something wrong. It is known that, for example, in the process of error correction, it seems subjective that “there was a lot of work”. However, when looking at diffs, it turns out that one or two lines were actually corrected. Watching also allows you to eliminate garbage from commits — for example, temporary debug statements and empty changes (adding and removing spaces). It often happens that only whitespace changes remain in this or that file - they must be rolled back to their original state (svn revert). In the endgame phase, watching changes before a commit is strictly required.
Separately, it is worth mentioning the correction of alignment and spaces in the files. Such an operation (if it is strongly needed) must be committed on its own. In the endgame phase, under no circumstances should you commit whitespace changes along with significant changes.
Also smaller rules:
- when renaming a file and making changes to it, you should separately commit renaming, and separately change;
- when adding a third-party component to the project code (the Rails plugin, the prototype.js file), they should be committed separately;
- do not leave commented lines of code - for storing old versions, of course, there is a version control system (in some cases with a one-two-line fix, this rule may be violated).
Questions, suggestions, suggestions?
PS We have a whole bunch of vacancies: undev.ru
The examples will use Subversion, although all of the recommendations are fully applicable to any other version control system.
Roughly divide the project development phases into three - a debut, middle game and endgame.
')
In the debut of the new code is written in huge chunks, often whole chunks of the system are transferred from place to place. The release is far, there are no special requirements for the general state of the system. It is even possible to break the system down to some limits.
In the middlegame, the system as a whole has stabilized, the product is nearing release. Refactoring is clearer, although sometimes quite extensive. At this stage, it is already expected that the system as a whole works - at least, a broken repository is reproached.
Finally, the system enters the endgame just before the release and immediately after. For web applications, new relatively small features are constantly being added, and major changes are first tested on branches. For more traditional applications, on the contrary, a branch is created for maintenance releases, and the development of the next large version continues on the trunk.
In the debut, commits are made relatively rarely. For example, creating a new class, a new relationship, or writing a preliminary version of a fairly large piece of code can be a good commit point. For some programmers, especially high-performance ones, there may even be a convenient commit tied to time — two or three times a day. The repository can be in a fairly “parsed” state, to the extent that the extracted code is simply not compiled.
In the middle game, the stream of commits is much more structured. Commits more often, and are tied only to structural changes. Most commits are no larger than average. Good points for a commit are: closing a task or a bug in the tracker, refactoring, adding or fixing another feature. The repository is almost always in a more or less efficient state, and broken repositories are reproached (and the fact of breaking causes a karmic blow).
Finally, in the endgame, commits are somehow controlled rather tightly. In general, the very fact of working on a project in this phase should have some justification - usually this is a bug or an important feature postponed after the release. The commits on this phase quickly go to production, so they: are clearly tied to the bug tracker; relatively small; well documented; well tested and thought out - introducing additional bugs at this stage causes a serious karmic blow).
Based on many years of experience in developing, supporting and reviewing fairly large software systems, we have compiled a set of important recommendations. They are designed, firstly, to increase the depth of awareness when working on a project; secondly, to facilitate the work of the code-reviewer - and therefore the likelihood of finding a bug or a flaw; thirdly, to alleviate the fate of a person who will be forced to understand the history of a project sometime in the future (often the author of current commits himself will be such a person).
In general, in our opinion, the depth of understanding by the programmer of the project he is working on strongly correlates with the quality of the commits that he does in the process.
The commits should be logical. A commit must correspond to one structural unit — a new file, a new block, a new employee, a new class, a new relation, a new feature, a closed bug, a single refactoring, a chapter of documentation, a correction of documentation, etc.
The meaning of this requirement is to play for the good of the future. By observing it, you can be sure that a particular commit in a stream is either safe (for example, fix documentation), or it can be freely rolled out with confidence that exactly one logical change has been rolled back. This is an invaluable property for binary search errors. Moreover, it is obvious that logical changes are much easier to see in the code review process.
The commits should be tested (with the exception of the debut phase). Ideally, the change is generally tested by unit tests. In the extreme case, a commit must be checked at least once - even the most innocuous corrections can lead to disastrous consequences for karma. Of course, a commit should be syntactically valid - fix to fixes a blow to the image.
Commits should be well documented. We are silent about empty journal messages - this is generally the worst that can be. This is permissible only for employees who are completely distant from programming (such as designers) who hardly agreed to your subversation at all. And it makes sense to work on the cultural level, even such people! A commit with the description of “Fix” or “Fixed” is allowed only if there are no more than two or three lines in the differential. If more - you need to write in more detail.
Separately, it is necessary to consider the issue of a journal message consisting of several unrelated items. It is clear that such a message is a symptom of a true problem — one that your commit consists of several unrelated parts. The best solution in this case would be to cancel the attempt of the commit and try to split it into several parts.
In the endgame phase, poorly documented commits should be strictly censured. Also, if a commit is a closure of a bug, then the bug number should be mentioned in a journal message. The same Trac integrates with Subversion, identifying ticket numbers in journal messages and automatically turning them into appropriate links.
Before committing, you need to view diffs, as well as keep track of new files that have not been added to the repository, and add them. If you use TortoiseSVN, then it has a special menu item called “Check for modifications”. The command-line client for this purpose are the svn st commands (there are no dangling files) and svn diff | more (for reviewing changes before a commit).
This rule must be observed in order not to accidentally commit something wrong. It is known that, for example, in the process of error correction, it seems subjective that “there was a lot of work”. However, when looking at diffs, it turns out that one or two lines were actually corrected. Watching also allows you to eliminate garbage from commits — for example, temporary debug statements and empty changes (adding and removing spaces). It often happens that only whitespace changes remain in this or that file - they must be rolled back to their original state (svn revert). In the endgame phase, watching changes before a commit is strictly required.
Separately, it is worth mentioning the correction of alignment and spaces in the files. Such an operation (if it is strongly needed) must be committed on its own. In the endgame phase, under no circumstances should you commit whitespace changes along with significant changes.
Also smaller rules:
- when renaming a file and making changes to it, you should separately commit renaming, and separately change;
- when adding a third-party component to the project code (the Rails plugin, the prototype.js file), they should be committed separately;
- do not leave commented lines of code - for storing old versions, of course, there is a version control system (in some cases with a one-two-line fix, this rule may be violated).
Questions, suggestions, suggestions?
PS We have a whole bunch of vacancies: undev.ru
Source: https://habr.com/ru/post/20248/
All Articles