Robert Zaremba, https://scale-it.pl

How we work

Development Team Agreements

version 2.1


http://bit.ly/dev-how-we-work 

.


1. Intro

This document presents the set of objectives for the development team. It extends the How We Work “affection”.

2. Extreme Programming

We will practice Agile Extreme Programming.

Agenda:

  1. Weekly sprint planning: Monday morning.
  2. Daily updates. Every day when you start a work.
    As per distributed environment we will do it in the #dev-daily-updates channel.
    Everybody, every working day should post there following message:
    What I’ve done. What I’m doing / plan to do. What are the blockers. What I found interesting (if any). Update the task board!
  3. Keep on track with the sprint plan.
    Always check the task board (trello) and update once you achieve something. Comment if something is badly designed, doesn’t work, there is a blocker or when something is just great!
    Work principles: Be proactive. Measure. Communicate!
  4. Weekly sprint review. Friday morning.
  5. Retrospective every 2 weeks.

3. Communication and collaboration

In a distributed environment communication is crucial. Without a good communication we will experience bad process. To ensure a good communication, all team members consent to:

  1. Be open — always inform your colleagues about your issues, when do you finish your work, when they can count on you. Always inform about non-default schedule.
    It’s normal that you have private things to do. But don’t hide it. Others may expect something from you. Let them know that you won’t be available (if it’s something longer).
  2. Plan — if you have impediments, issues or something which puts the delivery into a risk - plan it. Be proactive. Think about it. Discuss it. Best if you can propose some preliminary solutions.
  3. Be honest! This is very important. We can’t plan or communicate well if you are not honest. If you have a big problem -- let us know. Otherwise we will fail to make a plan and work as a team. Remember the team performance depends on you.
  4. If you have a personal issue — talk about it with the team manager. It’s better to solve things together.
  5. Be responsible.
  6. Provide daily updates (stand-up). Even if something was discussed, it’s better to repeat yourself in daily update then not doing it at all.
  7. Even if you didn’t do much - commit and push your changes.
  8. Every day update the task board. Move tasks or put comments.

Working together with a code.

If some feature development is dependent on other task which is work-in-progress, then don’t hesitate to do it in a single branch. It’s fair (and it’s part of git design)  to allow multiple people work on a single branch. This is crucial when you have an important task, and someone wants to collaborate, review our get your updates. Make it easy to allow others to collaborate.

4. Development Flow

  1. All code resides in repository assigned to the project.
  2. Follow the tasks.
  3. You should always have some ongoing task.
  1. All your ongoing tasks must have estimated due to date.
  1. Don’t hesitate to create new tasks - put them into the icebox (defined in FAQ below).
  1. Create a task for every feature / bug / ticket.
  1. Focus on your current tasks. Whenever you will find new thing to do, just add a task to the icebox section instead of losing a focus of a current task. We will plan it later.
  2. Communicate early and openly! Start from solution design before coding any complex task and present it. Make a functional review (no code review) for each bigger feature (>= 2 days) early - like in ⅓ of the work, where you will share your design and algorithm.
  3. Gitlab flow
  1. All features should be in branch
  2. Create meaningful names to your branch
  3. Always assign a reviewer to your code when doing pull-request (PR)
  4. Keep the features small and create PRs often (ideally 1-3 days, max 4-5 days).
  1. All packages / projects have to have integrated linters and test framework.
  2. We start each new API related task with a blueprint design. For the  REST we are using the API Blueprint. For other kind of APIS (like GraphQL or a simple package design) we use the the early code blueprint with interfaces. If the API design is not ready, push it and get early review! Say that it’s blocking. We prefer to spend more time on the design rather than spending too much time on rewrite.
  1. Frontend can use API Blueprint mock server to use it when implementing the frontend features before waiting for the backend.
  2. Backend should validate the response with the API Blueprint
  3. We use Snowboard as the rendering engine (documentation) and mock server.
  1. Make sure your editor has configured editorconfig support. The repositories contain .editorconfig file and it should be well respected.
  2. All feature / bug / ticket implementations should have corresponding tests. It is required that each new feature will have:
  1. unit test for all critical computation parts (frontend model and all backend packages)
  2. module test for backend functionality covering real world scenario.
  1. All code in the master branch must:
  1. Pass all tests
  2. Pass all linters
  3. Be well structured
  1. Take care about code quality. Check the sections below:
  1. What we use to preserve good code style
  2. What is a filename convention
  1. Choose wisely about function / variable names. Look below at Code Style
  1. What's in a name?
  1. Master branch is untouchable! Only merges from feature / hotfix branch are allowed.
  1. Implement all features in branches
  2. You can't rebase master and hotfix branch. You can rebase feature branches.
  1. Each application should have a README file which provide:
  1. Explain application purpose
  2. List all dependencies
  3. Explain the privacy / security mode (who and how can use it)
  4. Runtime requirements: can it scale, how it should be run, what are minimum computing resources required (memory, cpu, …).
  5. Links to the application design documents (if any).
  6. Installation instructions.

5. Code Review

The overall aim of the code review process is to increase the quality of the codebase.

Pull requests

When you finish implementing a feature make a pull request and ask someone for code review.

Reviewer Guidelines

Not 'does it work?' but 'is it easy to understand?' and 'is it simple?'

It is not the responsibility of the code reviewer to ensure that there are no bugs before code is committed, nor even that the code functions at all. These are 100% the responsibility of the person submitting the code. If the reviewer wants to undertake these checks then that is great, but beyond what we expect of code reviewing.

The code reviewer should primarily assess the maintainability of the code submitted, which means asking the following basic questions:

The enemy of both code maintenance and code robustness is complexity. Your task as code reviewer is to find complexity and complain about it.

The code reviewer can also assess by inspection other important aspects of the code, such as:

Caveat: as a code reviewer prioritize comprehensibility over efficiency. In other words, as is often said, we should aim to 'make it right, then make it fast' (not to try it the other way around).

Best Practices

There is a huge amount of advice regarding best practices, for example at: javapractices.com. We can't expect that submitted code is checked against all best practices. Here, for emphasis, are what we regard as the most important issues:

Length of code

Probably the most important issues to look at relate to code length:

Code organization

Another issue can be package structure. Each class should be organized into a comprehensible package structure. There shouldn't be too many packages, nor too few. As a rough guide, if a package contains less than 4 or more than 10 classes, it may be worth rethinking.

Naming

Another very important issue which is often under-rated is naming. Are packages, classes, methods and variables in your view, optimally named?

As a code reviewer, you might fear that you are being "picky" by complaining about names, but naming is a critical factor in the maintainability of code. If names are well-chosen, they serve as abbreviations of the functionality they name, which assists enormously in comprehending the whole. If a method is not well-named, one has to read each line of code to understand what it does, which is a very inefficient alternative.

Checklist

  1. Is the person submitting the code on the list of people who have confirmed transferral of copyright of code submitted?
  2. Am I having difficulty in understanding this code?
  3. Is there any complexity in the code which could be reduced by refactoring?
  4. Is the code well organized in a package structure which makes sense?
  5. Are the class names intuitive and is it obvious what they do?
  6. Are there any classes which are notably large?
  7. Are there any particularly long methods?
  8. Do all the method names seem clear and intuitive?
  9. Is the code [well-documented](http://www.javapractices.com/topic/TopicAction.do?Id=60)?
  10. Does it follow common Java code conventions?
  11. Does the code submitted introduce unnecessary dependencies which reduces the modularity of the codebase?
  12. Are there ways in which this code could be made more efficient?
  13. Are exceptions simply swallowed anywhere, rather than being handled explicitly?

5. Deployments

Normally we deploy only release branch, which should contain only validated and tested (all tests pass, manual regression tests pass) version from master branch.

Deploy only using OPS Code tools (provisioning) and buildbox environment.

FAQ

What is an Icebox?

Icebox is a place in our project management software where everybody can propose a task. It contains stories that have not yet been prioritized.

On most of our projects, we find ourselves discovering new stories all the time, based on the continuous feedback loop that Tracker encourages. Not all stories end up in the backlog, but it’s liberating to be able to capture new ideas and user feedback as they occur.

The downside of this is that the icebox, where all new and unprioritized stories live, can grow out of control very quickly. It’s not uncommon for longer lived, active projects to end up with hundreds, even thousands of stories on ice.

More information about it and related process you can find in the  Defrosting Your Icebox article.

Synchronizing: merge or rebase?

Read a great merge or rebase post by experienced people from Atlassian (the creators of bitbucket and tools for git and mercurial management). Summarizing:

When should I push my commits? What if I need to break in the middle?

Push when you finished your feature/mission or when you finished logical part of a task. Normally you shouldn't push every commit update, unless you are working with someone. Having lot of checkpoint might be good, but push only the good one (others just squash).

When you need to switch machine, or stop in some unfinished shitty place and you are not certainly when/where you continue then you can always make a side branch feature-x-shit-y (treat it as a private branch) and push it. When you come back to continue when you can squash fixes and middle-in-the-shit checkpoints and rebase on the head of feature branch. But it should be really rarely - because the missions should be short.

It is an easy recipe for efficient workflow, where we can efficiently debug the history.

When can I commit directly to master?

You can commit directly to master when your change:

Usually those cases are rare. Often you want to review your code and create a new branch.

If your change is small and fits in one commit than you can cherry-pick it from feature branch instead of merging (read the last, mentioned in previous question tip )

Are you afraid that Git lost your commits?

That is not the case. git preserve the branch you were on until the rebase successfully completes. In the meantime you can always do a "git rebase --abort" to undo the rebase. After it commits, you can use git reflog to see all of the commits that were supposedly "lost" or "uncommitted" from any recent rebase (since the last garbage collection which for me is less than once a month). You can easily create a branch for any of those commits using "git branch foo " and look at it, switch to it, etc. I have NEVER lost a single commit doing a rebase after many years of using git heavily. When there are no conflicts (usually the case) then everything happens totally smoothly. When there is a conflict, either I easily resolve it or else I "git rebase --abort" and react to the kind of conflict I got such as reordering or squashing the commits using (git rebase --interactive). If several of my commits conflict, I may use "git merge" so that I only have to resolve conflicts once, in which case the noise in the history is worth it.

What is our branch naming convention?

[app_name]-[fix]-<update-description>

Examples:

What to do when you can't find settlement with reviewer?

Call him or find other developers who will make a quorum.

How to make review issues

While commenting / discussing a pull request we need to track what needs to be done to accept the pull request. Asana is our tool to track all tasks so it reasonable to use it for review issues (issues from particular review) tracking.

By adding a subtask to review task:

Good habit is to put a link to a commit/comment page related to this issue.

What are non-blocking review comments?

When reviewer:

then such reviews are non blocking to approve the pull request. Force reviewer to add constructive comment. Otherwise when it doesn't help ask someone else to approve such commit.

Code Style

All pushed code should confirm to the code style:

To meet all these requirements you should:

Choose good names for the code:

Code comments & docs style

Code is our common good.

  1. All the style is verified by lint tools (yes it works also for comments).
  2. Comments should be compact and readable.
  3. TODO, FIXME sections should be linked with corresponding tasks on Task Management Tool.
  4. Good code brings better info than any comments.
  5. Prefer good functions, variables names then explaining cryptic things. Of course you can use short variables.
  6. If you need to explain most part of the function, then usually the function is wrong constructed. Try to refactor.
  7. If there is already a place for a comment domain, then don't repeat that part in other places (eg: React js component arguments should be documented in propsType rather than @props).
  8. If you need to comment function arguments in Go then don't double info about argument type. It is already there (in a function declaration)! Proposed syntax: @arg-name description

// LoadConfig opens and parses a yaml application config.
// @path principle path to read a config. If it's an empty string,
//   or it points to not existing place then it will try to read
//   config from "/etc/.../config.yaml"
// @return error if can't read or find a config file.
func LoadConfig(path string) (config *yaml.File, err error)

What is a filename convention?

For .go and .py files:

What is about code generation?

Some code is generated by separate program - for features which are better resolved by a machine then a human. In particular this goes to:

Whenever touching develop branch make sure that code generators will not introduce new changes (all potential changes should be committed). All generators should have tests to check if everything is committed.

When changelog should be updated?

Changelog (web/changelog.md) presents a broad outline of features changed / introduced in application. Whenever you make a new feature or make a significant feature modification - which will change a use case then you should include it in changelog before doing PR. We are not going to list all things there - these we have in commit log.

What next?

Become ninja

Resources

  1. Gitlab handbook