Code delivering workflow

Intro

The current article is the continuation of Task delivering workflow which represents a global view of team work organisation from the project manager point of view. It describes the interaction of developers with the code base to give a simple and effective way of code delivery from concept to production.

Our actors in this process are:

  • developer: someone who is supposed to implement the idea
  • QA specialist: someone who checks the code
  • CI robot: a process that runs tests and static code checks and reports the results
  • PM specialist: someone responsible for giving insights about tasks, managing their delivery and representing the results

Our tools in this process are:

  • Task tracking system (e.g. Jira) - software with tasks information and their status tracking
  • Code base or repository (e.g. github.com) - online software where we store our code and make comments about it
  • CI - software for triggering automatic tests and static code analytics
  • Version control system (e.g. Git) - software that contains the code of the project as well as all historical changes of it from the beginning to the current moment. It is closely related to the code base which is just an online storage for the code as well as a working space for making comments during the code review

Process description

Start of implementation

Developer wants to implement a task. He changes the status of it in a Task tracking system to "In progress". He switches to the root version of the code in the version control system. In Git we do it like this:

//cd to the code folder
git checkout master
git pull

The command above will fetch update the local version of the code on the current machine, downloading e.g. the accepted changes made by other developers.

We branch from the root version of the code, which means we create a secure copy of the root version, so all changes made here won't affect the root version as well as code versions of other developers (see the picture below):

Let's do this in git:

//this is literally saying: "create a branch with name 222_Implement_contact_form_submission inherited 
//from master (root branch)" and switch to it
git checkout -b FOL-222_Implement_contact_form_submission master

The proper branch naming is very important: the common pattern is task id + short feature description. Having task id in branch name allows to connect code base with the task management system and keep branch lifecycle in sync with the task flow. Short feature description is needed for the case, when a task requires more features to be coded. 

A general rule of thumb is however: 1 task - 1 branch, as a branch is the representation of the task idea in the code. If you use Jira as task tracking system you can also connect your tickets to the branches in bitbucket.org see (https://confluence.atlassian.com/bitbucket/connect-bitbucket-cloud-to-jira-software-cloud-814190686.html). 

Implementation

Now the developer is free to write the task code without being afraid to break the existing root version. It's reasonable to pack changes into "commits" or logical groups of changes. What belongs to one group? Usually I would recommend to create commits for the code you can easily revert/remove/rollback without affecting other changes you made previously. 

Let's look at some good and bad commit identifiers:

Fol-222_Regex to filter out invalid urls in the form

Fol-222_Image upload form

Fol-222_Fixing wrong escaping

Fol-222_Cycle detection algo for categories

The ids should have ideal wording as those are just hints for you to find the necessary piece of changes or give some reasoning of code changes for future discoveries (like answering the question why the hack my function was changed???)

Please note the task id in the commits. It's very helpful when you try to figure out, why some changes in the concrete code place were introduced to find out task. In many cases it helps to understand the history of the task.

There are also some bad commit ids:

Fixing it //no task id, very general sentence

Fol-222 Cycle detection and file management and added file uploader for js //too many features in one commit, imagine you want to rollback the cycle detection (try a better approach), but keep other changes. It will be impossible!

Fol-222 Bad code removed //why it's removed, why it's bad

We create commits in Git like this:

git status //very important to check the status before making a commit

Changes not staged for commit:
  (use "git add ..." to update what will be committed)
  (use "git checkout -- ..." to discard changes in working directory)

        modified:   src/Kernel.php
        modified:   src/Migrations/Version20190324012814.php
        modified:   tests/Form/CustomerTypeTest.php

//we don't add all files to the commit but only the ones related to one feature e.g.
git add  tests/Form/CustomerTypeTest.php src/Migrations/Version20190324012814.php
git commit -m "Fol-222_Fixing_failing_integration_test_because_of_missing_column_id"

Look at the code above - we didn't add src/Kernel.php to the commit because it was related to some other feature so we keep commits clean. 

The usual workflow is to create many different commit messages during the working day. It is also recommended to merge changes from the root version into your branch version to keep your code up to date with other developers. Doing this 2-3 times a day would be enough.

To do a merge first make sure you don't have any changed files which were not included into commits:

git status

If you see:

Your branch is up to date with 'origin/Fol-222_Fixing_failing_integration_test_because_of_missing_column_id'.

nothing to commit, working tree clean

It's save to merge with the base version.

If you see:

Changes not staged for commit:
  (use "git add ..." to update what will be committed)
  (use "git checkout -- ..." to discard changes in working directory)

        modified:   src/Kernel.php
        modified:   tests/Form/CustomerTypeTest.php

This means you have changes not included into commits. If you think it's too early to do this, stash the changes:

//we add first all changes to the potential commit
git add .
git stash

You can check git status again to make sure there are no pending changes. Let's merge base version into our branch:

//switch to the master branch
git checkout master
//load changes from the remote repository
git pull
git checkout Fol-222_Fixing_failing_integration_test_because_of_missing_column_id
git merge master

This will bring the accepted changes from the root version happened after you created a branch. It's very a bad practise not to merge the root version for a long time as with frequent merges you can detect early possible incompatibility or interface changes affecting your current work, so you can react accordingly. 

At the end of each working day we send our day work to the remote repository. This is very important for several reasons:

  • you backup your daily work, in the case when something happens with the computer you might loose it which is a very nasty situation
  • you notify other developers about your work so they can identify possible incompatibility issues with their work early
  • you trigger CI to check your code early as it's usual to attach CI activity to remote pushes of new changes
  • you can get sick (or have another reason for being absent) but your team will still have an access to your latest work

Let's do the backup:

git push origin Fol-222_Fixing_failing_integration_test_because_of_missing_column_id

Please note that you need to have at least one pending commit to make this command work as expected otherwise nothing will happen.

 Finishing implementation

At this stage the work is ready and can be checked. A developer sets the task to status "Done" and creates a pull request, which is a set of all changes made for the task implementation. In github.com it looks like this:

Pull requests are actually the changes that a developer proposes to merge into the root version. At this stage we usually finish the implementation a go the the next task, which is taken over by quality assurance tools. 

Code evaluation

We usually notify our colleagues to review the proposed changes (e.g. by sending pull request link in the team chat). In the meanwhile a CI process is triggered. If it fails, the developer is supposed to do the fixes, in case of big troubles, the task should be taken back into work. 

Code reviews are ok to be early before any QA or CI process is started. Sometimes developers are not sure about their solutions, so they give some proposals to the other team members via pull requests. 

In case with final code reviews (before QA), we either fix suggestions or do some disputes about possibility to implement them. In case of big problem discovery, we put the ticket back to development. Anyway it's pragmatic to give pull request for the code review early before we go too far into a wrong direction.

Usually we expect to have at least one approve of a reviewer and no open comments from other participants. Anyway it's not right to ignore some suggestions, in that case QA should regulate this.

When and only if CI is green, QA can do some manual checks. We usually report all bugs in the comments to the tasks. We do screenshots and describe exact scenarios how to reproduce the problem. We also check critical logs, database load for new logic, disk size etc. For any failures task is pushed back to reopened state. New feature suggestions which are discovered during testing are defined as followup tasks (since it's not pragmatic to play a "ping-pong" specs game.  

We expect from developers to postpone their current tasks (if not urgent) and fix the reopened ones in the first priority. 

It manual tests didn't show any problems, we merge the pull request into the root version (master branch in git). This means that the changes will immediately become available for other developers (if they do regular merges with root), but doesn't mean that the features will appear in production. We change the status of a task to "Ready for deploy".

We want to keep the latest changes in root to have a chance to discover bugs in a more global context of features combinations. 

At the end we trigger a deploy script which brings the latest root changes to the production. At this point we mark the task as closed. 

Task flow design

Our usual task flow looks like this:

Our task states are:

  • Backlog: Every new task is pushed to the backlog. It's a status for all tasks, which could be possibly implemented, but not yet selected for being done
  • Todo: A set of tasks which should be done as fast as possible. Those are selected, evaluated and have an implementor attached
  • In Progress: As the name suggests, here are the tasks in work
  • Done: Implementation is finished, the tasks should be evaluated
  • Requirements check: QA process to make sure, tasks implementation is free of problems
  • Waiting for related tasks: Some tasks are dependant on each other, e.g. classical web apps, where frontend is implemented on the backend and vice versa
  • Reopened: A problem in a task was found, so we cannot deliver it to production
  • Passed requirements: All tests are finished, the tasks here are ok to be delivered
  • Ready for deploy: We selected such tasks for the deployment (and some others keep in the previous state for some business reasons)
  • Released: The tasks code is on production
  • Closed: The task passed acceptance tests on production

Conclusion

Organised work with code should be an essential part of the task delivery process. We keep unchecked versions of it in branches and try to have a clean version of root without potentially buggy code. Once the code passes code reviews, CI and QA, it can be delivered to the root. Since all developers are expected to keep their versions in sync with the checked code changes, we have a chance to test the new code in more contexts. At the end it's delivered and the task is closed which corresponds to the final aim of the whole process.