Code Review Process at Trendyol Android Team
At Trendyol, code reviews play an important role in development cycle and keep improving since adopted. As we all know, code review has advantages like finding bugs early, having consistent codebase and sharing know-how etc. You can find plenty of blog posts even books about code review checklists, why to code review, code review effectiveness, best and worst practices.To sum up, this blog post mainly focuses on our review practices as Trendyol Android team. I will try to explain our motivation, most important points for us and basic rules while performing reviews.
Motivation
Basically, we adopt code review process in order to take advantage of its positive effects on team culture. Here is the list of items we benefited from:
- Team size: Currently Android team is divided into two sub teams which are named as discovery and checkout. There are 11 developers in total and having more team members is planned for the future. Therefore, it is really important for us to be on the same page in a development process. Thanks to code reviews, both sub teams know about team specific features.
- Consistency: Readable and maintainable code is one of the most important things we care as a team. With code reviews, we have consistent code base which makes code easier to read and understand. This also, helps new developers to get accustomed to codebase easily.
- Potential bugs: Generally, architectural or performance concerns and potential bugs are easier to be detected by reviewers. In this way, we can prevent potential bugs during code reviews even if it is overlooked in development.
- Knowledge sharing: As an Android team, we value learning new techniques and improving our knowledge. Both committers and reviewers can learn from code reviews. Committer can use different or more efficient technique which some reviewers doesn’t know about. Additionally, reviewers can suggest methods which improve the change done.
- Social bounds: During code reviews, team members constantly interact with each other. Thus, positive communication makes social bounds between team members stronger.
Basic Steps for Code Reviews
As Trendyol Android team, our release management workflow is based on Gitflow and repository management is handled by using Stash. If you want to learn more about our branch management and application life cycle, you can take a look at posts below.
We perform code reviews by opening pull requests on Stash, too. As we all know there are some basic best practices for code review and meaningful naming, readability, following architectural patterns are just few of them. Definitely, these practices are taken into consideration both on development and review phases in Trendyol. In addition to these, we set a guideline which we think beneficial for our development cycle. In this section, I will mention these practices, briefly.
- Branch name: When we create feature branches we use the following convention SPRINT/ISSUE_KEY/FEATURE_NAME. This makes our QA team to follow features easily and we also use this pattern in our automation process.
- Commit messages: We care about meaningful and simple commit messages. The following example is used for commit messages.
- Unit test: Unit tests are added before opening a pull request and all test suites are run as automated.
- Pull request scope: We try to open pull request as small sized as possible. If there is large scoped feature, as we mentioned earlier posts we create DEV_PROCESS branches.
- Coding conventions: We have coding conventions like max line length, meaningful naming, method length and so on. We are working on a lint rule set and we aim to include this rule set to our automation process. Also thanks to these rules, reviewers can focus on more crucial parts of pull requests.
- PR comments: Understandable and informative comments are valued by our team. Therefore, we try to be as clear as possible and explain why this change is preferred when we comment for pull requests and we care to share a link if it is necessary and beneficial.
- Commented out code: Having a clean repository is one of the important things for our team. To manage this, we avoid pushing commented out code to repository unless it is necessary. In necessary cases which we are going to uncomment that section in the future, we always use TODOs.
- Stash tasks: When reviewing if we leave a comment which requires a change which is not optional, we also create a Stash task. Reviewer can mark pull request as needs work but this is up to reviewer. Committer needs update this pull request and close the task.
- Architectural patterns: Trendyol Android application is developed in MVVM architecture. When we open pull request and perform reviews we follow this pattern. You can read more about our architecture from following blog posts.
- Complex logic: As we all know sometimes features can require complex logics or tricky solutions. In such cases, we give an explanation by leaving a comment.
- Merge conditions: In additions to all conventions which are listed above, we have some basic steps to see merge button enabled in Stash. First, all open tasks needs to resolved. Secondly, all unit tests needs to be succeeded. And lastly, required approve count needs to be satisfied. Totally minimum approve count is 4 in order to merge a pull request. As I mentioned earlier, there are two sub teams as checkout and discovery. A pull request must get at least 2 approves from related team members. Therefore if a feature is pushed from checkout domain at least 2 members from checkout team need to approve that pull request. These approve counts are determined based on our current team size and they are going to be updated when the new team members are joined.
In this blog post, I mainly focus on presenting our take on code reviews and what we care most. If you want to learn about our architecture, our conventions and our rules and more you can take a look at our guidelines in GitHub below. Thanks for reading and I hope you can find some useful parts for your own review processes. Let me know if you have any questions/comments, I would be happy to hear your thoughts!
