Skip to content

Latest commit

 

History

History
142 lines (110 loc) · 5.71 KB

CONTRIBUTING.org

File metadata and controls

142 lines (110 loc) · 5.71 KB
Raw

Contributing

Introduction

This document introduces how to minimize the friction between contributors. It is mainly written for developers.

So if you are a new developer in our team, welcome! Please read carrefully do not hesitate to reach out if you want to discuss something about this document.

  • coding style
  • git / deploiement workflow

Coding Style

Syntax

The code should follow the following guidelines:

https://github.com/bbatsov/clojure-style-guide

Mainly it states that the syntax should be the one provided by default with emacs. Most (every) developer use emacs right now to contribute.

Guidelines

The motivations for these guidelines are code readability and also management of code complexity:

  • Select meaningful names for functions and variables. Avoid acronyms that aren’t commonly known.
  • Prefer rewriting code to make it clear over adding comments. If that cannot be achieved, add comments.
  • Limit the number of functions invoked in a single line of code to five.
  • Limit the nested depth of a function definition to four. In order to achieve this it may be necessary to introduce local variables to hold intermediate results or extract code into additional function definitions.
  • Only use anonymous functions for short function definitions that fit comfortably on a single line. Otherwise create a private, named function.
  • Do not write anonymous functions that have arguments that are anonymous functions.
  • Make an effort to limit the depth of the code. Mainly, you should reach a lib or core function as soon as possible while following the functions of a namespace. Other member of the team should not need to open more than three namespaces at a time to reach all the functions necessary to understand your code.
  • Do not use loop/recur unless you can’t use reduce. Do not use reduce unless you can’t use a combination/threading of functions.
  • Never use defmacro unless you really can’t use a function and every other member of the team couldn’t find a better way.
  • Never use defmethod.
  • Only use defprotocol for declaring TK services. Never use it unless you can’t do otherwise. Most of the time using case and/or cond will be preferable.
  • Try to avoid the use of core.async there is certainly an easier, simpler and less bug prone way to do what you intend to.

Backward Compatibility

We work in a production environment. Once a code is in production, people are using in some way, we can’t break it.

Try to make your code upgrade proof. Inspired from (Spec-ulation talk of Rich Hickey https://www.youtube.com/watch?v=oyLBGkS5ICk). Ask yourself the questions:

  • “Will we be able to upgrade without downtime?”
  • “Will it be easy to put that new code in production without breaking the service?”
  • “Will that change imply to run a data migration in production?”
  • “Will that change break the workflow of any client?”
  • “How easy will it be to upgrade the configuration?”
  • “What will occur if the 3rd party change slightly their data format?”.

For example: notice that adding new fields to the configuration is not as expensive as modifying it or removing a key, because it will force that the deploiement of the conf and of the code to be synchronized. While if you can add new fields, you can upgrade the configuration before upgrading the code in production without any downtime.

  • For most spec/schemas, make them as permissive as possible. For example (defschema Conf {:port s/Int}) is not as permissive as as (defschema Conf {:port s/Int s/Any s/Any}). Because if the configuration change we would be able to add new fields to the configuration without breaking the running version.
  • Only add new fields, new values. Because modifying the type of a value or removing a field will break between different version of the code. 
    OK:     (s/enum :a :b) => (s/enum :a :b :c)
NOT OK: (s/enum :a :b :c) => (s/enum :a :b)

OK:     {:a s/Int} => {:a s/Int :b s/Bool}
NOT OK: {:a s/Int} => {:a s/Bool}

Git / Deploiement Workflow

In our team, GitHub is currently used both as a developer tool and as a task management tool. This document only focus about the developer tool aspect.

Git / Github

Our workflow is close to the Gitlab flow (see: https://about.gitlab.com/2014/09/29/gitlab-flow/)

Init your env

  1. Clone the ctia repo in github to have your own clone say (where johndoe is your nick): https://github.com/johndoe/ctia.
  2. Add a tg remote for the main threatgrid/ctia repository: git remote add tg https://github.com/threatgrid/ctia
  3. ./init-local-git-config.sh

You shall now have:

  • a default git commit template
  • an alias: git synctg that shall sync your origin/master with the threatgrid/master correctly.

You can test everything is working correctly by doing:

> docker-compose -f containers/dev/docker-compose.yml up -d
> lein test
> lein run -m ctia.main

Adding a new feature / fixing an issue

  1. You should have an open issue in github with number #XXX.
  2. git checkout -b issue-XXX-issue-short-description
  3. work… make a some git commit; the first time do not use the -m option, then for all other commits, you shall use a git commit -m with short messages (<50 char is best).
  4. Optionally clean up your git log history by doing a git synctg and then a git rebase -i master. Do not remove the first commit message with the Release block.
  5. Test locally ./build/compile.sh && ./build/run-tests.sh && lein tk
  6. git push -u will push and create the branch on github
  7. Open a PR. In the PR reference the issue, follow the instructions of the template.
  8. Make changes according to PR feedbacks
  9. Either use the Squash & Merge button in github or manually rebase.