April 5, 2018 at 11:43Code Conventions: how do we maintain the fast pace of developing a PHP project
Hello, Habr. My name is Evgeny Udodov, I am a co-founder and technical director of Roistat. I want to share our experience in developing a large and complex product - analytics system.
TL; DR : We posted our Code Conventions on github and talked about how to put it into practice in an article.
When developing large products, there is a common problem - over time, a lot of legacy code accumulates, tasks become slower and slower. Also, with the growth of the team, developers begin to write code in different ways and the lack of uniform rules can lead to conflicts and disputes.
Over the 4 years of the existence of our project, we have made more than 20,000 Pull Requests (hereinafter PR) and under the cut I will tell you how we solved these problems.
I tried to write the most useful article. You will be interested in it if you have encountered the problems listed below.
If you are a technical director or technical expert:
If you are a product manager:
At the beginning of 2014, only 2-3 developers worked at Roistat, but the first problems started already then. We are faced with the fact that each developer needs to explain the same mistakes. And with the growing number of developers, this situation has only worsened.
Also with experienced developers, there was another difficulty. They each had their own accumulated experience and their own understanding of how to build the architecture of the project. Very often their opinions contradicted each other.
Similar problems accumulated like a snowball. It was clear that if this continues, the development will lose speed, flexibility and each release will be given through pain.
First, we discussed the problem areas and agreed on words about what we will do in such cases in the future. Naturally, this did not work. After some time, everyone had a different idea about what we agreed on earlier. Also, new developers did not know about these agreements.
Then we thought: “How to fix this information for everyone in the same way?” We tried to record training videos in which we talked about the architecture of the project and the basic principles of our development. But this format was very inconvenient. Video is difficult to fix or overwrite. During the active growth of the project, new situations and new problems constantly arise that require either new agreements or changes to old ones.
Then we began to record the results of our discussions in a separate document. At the start, it was just a few suggestions. But we agreed that at Code Review, developers should not impose their personal opinions on code. Instead, they should refer to this document and point out inconsistencies with it. If it does not have the necessary rule, then first we must discuss the problem, jointly work out a solution, write it in a document, and then return and continue Code Review.
And it worked. Problems with different understanding of the agreements began to be minimized. In the document, we wrote detailed explanations with code examples so that no one could interpret them differently. Each new developer in our team could immediately understand what was required of him and how it is customary for us to act in various situations. Code Review, instead of endless discussions and disputes, has become a fairly transparent and pragmatic process.
After six months of work, this document already contained a lot of useful information and became an indispensable tool in the work of the team. We called it Code Conventions (or Code Conv for short).
Code Conv - these are the rules that must be followed when writing code. We distinguish between Code Style and Code Conv. For us, Code Style is the look of the code. That is, the placement of indents, commas, brackets, and more. And Code Conv is the semantic content of the code. The correct action algorithms, the correct meaning of the names of variables and methods, the correct composition of the code. Compliance with Code Style is easily verified by automation. But in most cases, only a person can verify compliance with Code Conv.
We take the following approach when using Code Conv.
For the developer:
For Code Reviewer:
In order to make it clearer what I'm talking about, here are a few examples of how this document looks in reality.
There is a table of contents at the beginning of the document. Over time, information becomes more and more without a table of contents - nowhere.
We have 3 main sections: values, general principles and specific rules.
Values are business requirements for a development team. The business does not know the details of which methodologies and principles are used internally, but it is important for it that the product is developed simply, quickly and does not become complicated over time. These values should be followed by the entire team in the development of new rules and to address issues that are not described in the rules.
Principles are ways to uphold previously described values. They are a little more detailed, contain the basic development methodologies and approaches that we are guided by. An experienced developer does not have to memorize the entire document. It is enough for him to understand these principles, and he can derive all other rules from these principles at any time.
Nevertheless, values and principles can be interpreted in different ways. To avoid misunderstanding, the document has specific rules and code examples that show how you can do it and how not.
There are a lot of such rules and they make up the bulk of the document. Thanks to them, order is maintained.
In practice, the application of the document is as follows.
The Code Review developer comments on portions of the code where he noticed Code Conv non-compliance. In a comment, he indicates which rule is violated. And optionally, if this is not obvious, adds an explanation of why he thinks so.
For example, in this screenshot we see that the DRY (Don't Repeat Yourself) principle is violated. Despite the fact that in this particular PR the developer wrote the code without repetition and, it would seem, used it only once, he nevertheless created a method that cannot be used in the future without additional verification. That is, he created a method that obviously requires repetition of code and logic.
Developers often try to push their code, which does not correspond to Code Conv, under various pretexts. They find a variety of excuses for their violations, but there are several of the most common.
Developers think that a single violation of Code Conv will not do any harm. Also, sometimes they believe that in some not very important place these rules do not carry values or are not critical. In response to this, we always refer to the theory of broken windows : "If one glass is broken in a building and no one replaces it, then after a while there will not be a single whole window in this building." If we make exceptions to the rules, then in six months we will find ourselves in a situation where the entire project code consists of such violations.
Developers often say that it will be normal to do for a long time, and the client is waiting for a solution to the problem, and the battle should be edited as soon as possible. But, firstly, Code Conv nowhere forbids doing the first iterations with simplified code. It's just that this code should still meet quality standards. Secondly, if you still need to violate Code Conv, then it is permissible to do this if the created entropy is not malignant (more on that below). But even in this case, everyone should have a common understanding that following Code Conv in this situation will really cost a lot more.
A few words about entropy. Entropy is the amount of information that a project consists of (information capacity of a project). If a developer writes code that adds critical information that every developer must know, then such entropy is malignant. Often such a code is a time bomb. That is, at the moment this ignorance may not affect anything, and in the future, critical errors may be made because of it. We call such entropy malignant. If ignorance of the information does not affect the work of other developers, then such entropy is not harmful, and we call it benign.
For example, if we make a separate microservice that will listen to some events and add statistics somewhere, then this service does not introduce malignant entropy. If other developers do not know about this service, then they will not make any mistakes, it will not affect their work. If it breaks, then no one will suffer except the owner of this service. So for such a service it is permissible to do the first iteration (like Proof of Concept) with low quality code. And then, if it needs to be developed, then it will be necessary to fix the technical debt laid down and work with it already by all the rules. As a result, Code Conv does not prohibit experimenting and making fast and easy concepts, if this does not affect the rest of the system.
Such an approach has allowed us to maintain a high development pace for more than four years, easily introduce new developers into the work, and not slow down over time.
Of the minuses, it can be noted that with this approach, Code Review takes longer than usual. Even small tasks can be delayed for several days or longer due to not the most critical errors.
But this minus is blocked by pluses, since the malignant code does not fall into the battle and all the “windows” remain intact.
We were repeatedly asked to share an example of Code Conv.
We decided to share with everyone and make it public. You can take it as is and use immediately. Or you can change for yourself. We are also open to PR and will consider your suggestions for improvement.
Link to our Code Conv
We are actively expanding the development team, so if our approach is close to you, respond to our vacancy or write to the mail .
TL; DR : We posted our Code Conventions on github and talked about how to put it into practice in an article.
When developing large products, there is a common problem - over time, a lot of legacy code accumulates, tasks become slower and slower. Also, with the growth of the team, developers begin to write code in different ways and the lack of uniform rules can lead to conflicts and disputes.
Over the 4 years of the existence of our project, we have made more than 20,000 Pull Requests (hereinafter PR) and under the cut I will tell you how we solved these problems.
I tried to write the most useful article. You will be interested in it if you have encountered the problems listed below.
If you are a technical director or technical expert:
- You have to spend a lot of time to understand what is happening in an unfamiliar project code.
- For each task, you have to point out the same errors to the developers.
- After discussing how to make a feature, the developer makes the PR not the same as agreed
- After an incorrect PR, the developer, making some mistakes, makes others
- You spend a lot of time training each new developer, even if he doesn’t later stay for a long time
- Senior developers come up with their own rules and impose them on less experienced guys
If you are a product manager:
- When discussing each (seemingly not the most difficult) feature, developers tell you that it is long and expensive to do it
- You roughly understand the principles of programming, but you don’t understand anything in the code of your project
- You have to wait a long time for new developers to keep up to date before they can complete complex tasks.
Start
At the beginning of 2014, only 2-3 developers worked at Roistat, but the first problems started already then. We are faced with the fact that each developer needs to explain the same mistakes. And with the growing number of developers, this situation has only worsened.
Also with experienced developers, there was another difficulty. They each had their own accumulated experience and their own understanding of how to build the architecture of the project. Very often their opinions contradicted each other.
Similar problems accumulated like a snowball. It was clear that if this continues, the development will lose speed, flexibility and each release will be given through pain.
Search for a solution
First, we discussed the problem areas and agreed on words about what we will do in such cases in the future. Naturally, this did not work. After some time, everyone had a different idea about what we agreed on earlier. Also, new developers did not know about these agreements.
Then we thought: “How to fix this information for everyone in the same way?” We tried to record training videos in which we talked about the architecture of the project and the basic principles of our development. But this format was very inconvenient. Video is difficult to fix or overwrite. During the active growth of the project, new situations and new problems constantly arise that require either new agreements or changes to old ones.
Then we began to record the results of our discussions in a separate document. At the start, it was just a few suggestions. But we agreed that at Code Review, developers should not impose their personal opinions on code. Instead, they should refer to this document and point out inconsistencies with it. If it does not have the necessary rule, then first we must discuss the problem, jointly work out a solution, write it in a document, and then return and continue Code Review.
And it worked. Problems with different understanding of the agreements began to be minimized. In the document, we wrote detailed explanations with code examples so that no one could interpret them differently. Each new developer in our team could immediately understand what was required of him and how it is customary for us to act in various situations. Code Review, instead of endless discussions and disputes, has become a fairly transparent and pragmatic process.
After six months of work, this document already contained a lot of useful information and became an indispensable tool in the work of the team. We called it Code Conventions (or Code Conv for short).
Code conventions
Code Conv - these are the rules that must be followed when writing code. We distinguish between Code Style and Code Conv. For us, Code Style is the look of the code. That is, the placement of indents, commas, brackets, and more. And Code Conv is the semantic content of the code. The correct action algorithms, the correct meaning of the names of variables and methods, the correct composition of the code. Compliance with Code Style is easily verified by automation. But in most cases, only a person can verify compliance with Code Conv.
We take the following approach when using Code Conv.
For the developer:
- The developer studies this document before writing the first line of code in our team.
- The developer must understand and understand the written rules. Just learning this document will not work.
- The developer strictly follows the rules of Code Conv when writing code.
- The developer creates a PR and immediately looks through it himself and checks for compliance with Code Conv
- The developer submits the PR to Code Review and corrects the errors and inconsistencies of Code Conv, to which he is pointed out and which he missed himself
For Code Reviewer:
- The reviewer checks the code for compliance with the rules described in Code Conv
- If during the Code Review a remark appears to the developer, then it should be accompanied by a link to Code Conv.
- If Code Conv does not have the required item, then a working group is going to develop a new rule and fix it in the document. After that, the reviewer returns to the PR check
Some examples
In order to make it clearer what I'm talking about, here are a few examples of how this document looks in reality.
There is a table of contents at the beginning of the document. Over time, information becomes more and more without a table of contents - nowhere.
We have 3 main sections: values, general principles and specific rules.
Values are business requirements for a development team. The business does not know the details of which methodologies and principles are used internally, but it is important for it that the product is developed simply, quickly and does not become complicated over time. These values should be followed by the entire team in the development of new rules and to address issues that are not described in the rules.
Principles are ways to uphold previously described values. They are a little more detailed, contain the basic development methodologies and approaches that we are guided by. An experienced developer does not have to memorize the entire document. It is enough for him to understand these principles, and he can derive all other rules from these principles at any time.
Nevertheless, values and principles can be interpreted in different ways. To avoid misunderstanding, the document has specific rules and code examples that show how you can do it and how not.
There are a lot of such rules and they make up the bulk of the document. Thanks to them, order is maintained.
In practice, the application of the document is as follows.
The Code Review developer comments on portions of the code where he noticed Code Conv non-compliance. In a comment, he indicates which rule is violated. And optionally, if this is not obvious, adds an explanation of why he thinks so.
For example, in this screenshot we see that the DRY (Don't Repeat Yourself) principle is violated. Despite the fact that in this particular PR the developer wrote the code without repetition and, it would seem, used it only once, he nevertheless created a method that cannot be used in the future without additional verification. That is, he created a method that obviously requires repetition of code and logic.
Frequent developer objections
Developers often try to push their code, which does not correspond to Code Conv, under various pretexts. They find a variety of excuses for their violations, but there are several of the most common.
“But once is it possible?”
Developers think that a single violation of Code Conv will not do any harm. Also, sometimes they believe that in some not very important place these rules do not carry values or are not critical. In response to this, we always refer to the theory of broken windows : "If one glass is broken in a building and no one replaces it, then after a while there will not be a single whole window in this building." If we make exceptions to the rules, then in six months we will find ourselves in a situation where the entire project code consists of such violations.
“But you need it very urgently!”
Developers often say that it will be normal to do for a long time, and the client is waiting for a solution to the problem, and the battle should be edited as soon as possible. But, firstly, Code Conv nowhere forbids doing the first iterations with simplified code. It's just that this code should still meet quality standards. Secondly, if you still need to violate Code Conv, then it is permissible to do this if the created entropy is not malignant (more on that below). But even in this case, everyone should have a common understanding that following Code Conv in this situation will really cost a lot more.
A few words about entropy. Entropy is the amount of information that a project consists of (information capacity of a project). If a developer writes code that adds critical information that every developer must know, then such entropy is malignant. Often such a code is a time bomb. That is, at the moment this ignorance may not affect anything, and in the future, critical errors may be made because of it. We call such entropy malignant. If ignorance of the information does not affect the work of other developers, then such entropy is not harmful, and we call it benign.
For example, if we make a separate microservice that will listen to some events and add statistics somewhere, then this service does not introduce malignant entropy. If other developers do not know about this service, then they will not make any mistakes, it will not affect their work. If it breaks, then no one will suffer except the owner of this service. So for such a service it is permissible to do the first iteration (like Proof of Concept) with low quality code. And then, if it needs to be developed, then it will be necessary to fix the technical debt laid down and work with it already by all the rules. As a result, Code Conv does not prohibit experimenting and making fast and easy concepts, if this does not affect the rest of the system.
Conclusion
Such an approach has allowed us to maintain a high development pace for more than four years, easily introduce new developers into the work, and not slow down over time.
Of the minuses, it can be noted that with this approach, Code Review takes longer than usual. Even small tasks can be delayed for several days or longer due to not the most critical errors.
But this minus is blocked by pluses, since the malignant code does not fall into the battle and all the “windows” remain intact.
We were repeatedly asked to share an example of Code Conv.
We decided to share with everyone and make it public. You can take it as is and use immediately. Or you can change for yourself. We are also open to PR and will consider your suggestions for improvement.
Link to our Code Conv
We are actively expanding the development team, so if our approach is close to you, respond to our vacancy or write to the mail .