Difference between revisions of "Software Development Guidelines"

From GreenVulcano Wiki
Jump to: navigation, search
m (Rationale written)
 
(Do not submit copyrighted material without permission)
 
(28 intermediate revisions by the same user not shown)
Line 1: Line 1:
== Rationale ==
+
== Introduction ==
 +
 
 +
=== Rationale ===
 
The GreenVulcano code base has been growing over time. While the code has traditionally been "reviewed" by one or two people (our historical "good dictators") before submission to public availability, this is becoming less and less the case due to our increasing core development community. Therefore, some self-discipline is needed in order to improve collaboration - and, sometimes, reduce the frustration due to switching from one style to the other between different source files (and often even within the same file).
 
The GreenVulcano code base has been growing over time. While the code has traditionally been "reviewed" by one or two people (our historical "good dictators") before submission to public availability, this is becoming less and less the case due to our increasing core development community. Therefore, some self-discipline is needed in order to improve collaboration - and, sometimes, reduce the frustration due to switching from one style to the other between different source files (and often even within the same file).
  
'''This is an open, ongoing, collaborative document!''' If you feel like a new guideline is needed for the project, then:
+
=== The ''Good Dictators'' ===
* Discuss it with senior colleagues to check that it actually makes sense, then
+
A '''dictator''' is really nothing more than a very senior technical lead. The reason why we use the term '''dictator''' is that we as a community take his/her advice very seriously. In fact, one of the few actions having serious consequences for a community member is to contravene an explicit dictator ruling or warning.
* Add it to this document, in the relevant section.
+
 
 +
Please note:
 +
* The role of our dictators is to preserve the overall quality of our software over time.
 +
* Besides spitting out new rules, dictators are usually willing to discuss non-trivial project-related issues. Do not feel afraid to ask if you feel you need some advice.
 +
* It's a role based on volunteering. Do not voluntarily waste teir time.
 +
* It's a hard job and takes a lot of time, along with a good deal of experience; do propose yourself as a dictator if you feel you can contribute on this account.
 +
 
 +
You can contact our dictators at dictators@greenvulcano.com [[mailto:dictators@greenvulcano.com]].
 +
 
 +
=== Structure of a Rule ===
 +
Each rule has, at least:
 +
* A '''Name''' (= Title)
 +
* An '''Importance''' classification:
 +
** HIGH
 +
** MEDIUM
 +
** LOW
 +
** COSMETIC
 +
* A '''Reason''' explaining why you have to comply
 +
 
 +
And, optionally:
 +
* An '''A.K.A.''' (also known as) for alternative naming of the same rule, or for easier understanding
 +
* '''Suggestions''' on how to comply with the rule
 +
* Known '''Exceptions''' to that rule. The fact there is a documented exception does not allow you to skip the [[#Exceptions_to_a_Rule|authorization]] process for exceptions.
 +
 
 +
=== Exceptions to a Rule ===
 +
Exceptions are generally said to prove the rule. So there must be a rule for exceptions.
 +
How exceptions are dealt with depends on the '''Importance''' classification of the rule:
 +
* '''HIGH''': exceptions are not allowed, in any circumstance. Ok, you can ask a ''dictator'', but expect a NO. If, by chance, you get a YES, or even a MAYBE, then probably the rule's importance needs to be degraded to MEDIUM. However, in case an exception ever gets authorized, the exception rule for MEDIUM applies.
 +
* '''MEDIUM''': exceptions are allowed only if authorized by a ''dictator'', and such authorization must be referenced in the code where the rule is infringed, along with the reason for infringement.
 +
* '''LOW''': exceptions are allowed only if authorized by the project's technical lead, and such authorization must be referenced in the code where the rule is infringed.
 +
* '''COSMETIC''': exceptions are not welcome, but tolerated if kept within reason.
 +
 
 +
=== Proposing a New Rule ===
 +
Write to the [[#The_Good_Dictators|dictators]] explaining your proposal, along with the reasons associated to it. It will be much appreciated.
 +
 
 +
== General ==
 +
This section contain general rules, which apply to all areas of software development. They usually fall into the widely accepted definition of ''Common Sense''.
 +
 
 +
=== Contribute to improve software that does not meet the guidelines ===
 +
; A.K.A. : Do not look away when seeing badly written code.
 +
; Importance : MEDIUM
 +
; Reason : Code tends to live long, especially foundation code. If it's badly written, it has the potential to undermine the overall quality of your software for a very long time (seasoned programmers certainly have their own anecdotes about this). So, whenever you encounter software that doesn't meet one or more of the guidelines in the following document, you definitely should take the time to make it better.
 +
If you don't feel comfortable modifying the code (because it's too tricky to touch it, or because you do not grasp the specific topics addressed by the software), or you just don't have enough time, open a ticket on the bug-tracking system, referencing the exact location of the offending code, along with your observations.
 +
 
 +
 
 +
=== Do not submit copyrighted material without permission ===
 +
; A.K.A. : Check the license of the software tools and libraries upon which you are basing your software.
 +
; Importance : HIGH
 +
; Reason : Multi-million €/$ lawsuits may arise when powerful companies believe other people have used their own protected assets (such as copyrighted software). It is therefore of vital importance that you make sure you have the right to use somebody else's work for your purposes. OSI-approved licenses ([[https://opensource.org/licenses]]) are generally OK - although they also have their subtle differences between each other - while software licensed under different conditions may be very dangerous. Do write a couple of lines to a [[#The_Good_Dictators|dictator]] if you are unsure.
 +
 
 +
=== Put a copyright disclaimer and software license in each source file ===
 +
; Importance : MEDIUM
 +
; Reason : It may happen that a source file is moved somewhere else from where it originally belonged. In such cases, the license originally ''shipped'' with the file might not be available anymore. Putting a short disclaimer on top of the file help keeping track of licensing conditions.
  
 
== Design ==
 
== Design ==
 +
This section covers common guidelines concerning the design of our software.
 +
 +
=== Clearly separate concerns in your design ===
 +
; A.K.A. : No business and presentation logic mix-up
 +
; Importance : HIGH
 +
; Reason : Whenever possible, our software has to make no assumptions on the context where it runs. As a matter of example: it happened that software originally targeting the desktop needed to be ported to the cloud - it was a bloodshed to separate the actual processing logic from "buttons", "text fields" and "properties".
 +
 +
=== Never hardcode settings or "magic" values ===
 +
; A.K.A. : You die if you do that
 +
; Importance : HIGH
 +
; Reason : Your code can (and will) be used in contexts which are totally unknown to you. It will need either to be totally context-independent, or to make all context-related aspects configurable.
 +
; Example: A <code>ConfigProperties</code> reader '''must''' have a constructor specifying "which" file to read.
 +
 +
=== Provide a reasonable default for configuration-based software ===
 +
; A.K.A : Convention Over Configuration
 +
; Importance: HIGH
 +
; Reason : We mainly develop middleware. This usually requires a lot of knowledge about the environment the software runs within, which means a lot of configuration. The user may easily feel overwhelmed by having to configure each and every bit. Therefore, when a "default" behavior just makes sense, in absence of a more specific configuration
 +
; Example: Besides having a constructor specifying which file to read, a <code>ConfigProperties</code> reader shall also have a constructor omitting such parameter, which shall search for "config.properties" in the classpath or current working directory.
 +
 +
=== Avoid references to your own workspace files ===
 +
; A.K.A. : no <code>/home/johndoe/projects</code> on SVN or GIT
 +
; Importance : MEDIUM
 +
; Reason : Every developer has his/her own settings and favourite IDE layouts. If such preferences are placed in a shared, versioned file, a ''commit war'' will inevitably start - causing frustration in the entire team/community.
 +
; Suggestion : In order to avoid this, only place ''common'', ''default'' settings in the versioned files, and provide a template for unversioned ''user properties''. Then use a smart configuration reader that allows ''user-specific'' properties to override the default ones.
  
 +
== Naming and capitalization ==
  
== Naming ==
+
=== Name classes, functions, and variables as the language of your choice indicates ===
 +
; A.K.A. : Keep consistent with the naming convention of the language you are using
 +
; Importance : MEDIUM
 +
; Reason : Staying consistent with naming helps to improve readability. There's no need for esoteric notations or conventions: all of us are nowadays using IDEs to help us tell static, pointers, constants and the rest apart.
 +
; Example :
 +
  (Java)
 +
  public class MyClass {
 +
     
 +
      private static final Object STATIC_FINAL_OBJECT = null;
 +
      private static      Object staticObject;
 +
     
 +
      private              Object instanceObject;
 +
     
 +
      public void myMethod() {
 +
          int methodVariable = 0;
 +
          // something else here
 +
      }
 +
  }
  
 +
  (Python)
 +
  class MyClass(object):
 +
      def __init__(self):
 +
          self.public_variable = 0
 +
          self._protected_variable = 1
 +
          self.__private_variable = 2
 +
     
 +
      def public_method(self, arg):
 +
          pass
 +
     
 +
      def _protected_method(self, arg):
 +
          pass
 +
     
 +
      def __private_method(self, arg):
 +
          pass
 +
 +
=== Use the native capitalization convention for the language you are using ===
  
 
== Formatting ==
 
== Formatting ==
  
 +
=== Indent consistently - four spaces, no tabs ===
 +
; Importance : MEDIUM
 +
; Reason : Using tabs makes your code render differently on different devices (e.g. some may have tab stops set at 8 spaces, while yours might be 4 spaces).
 +
 +
=== Remove unused code ===
 +
; Importance : LOW
 +
; Reason : Unused / commented code is confusing and complicates reading. Versioning tools are great at restoring deleted code, should you need it back. Unused code also includes unused import/include directives.
  
 
== Versioning ==
 
== Versioning ==
 +
 +
=== Do not commit broken code ===
 +
; A.K.A. : Do not break the software build.
 +
; Importance : HIGH
 +
; Reason : When you commit broken code (i.e. code that does not work properly, or - even worse - doesn't compile) you are negatively affecting the entire team's productivity. People might be blocked waiting for your fix in order to test their own work, and centralized tools such as code analysis, continuous build and integration testing '''will''' be unreliable for the hours or days to come.
 +
 +
 +
=== Do not commit untested code ===
 +
; Importance : MEDIUM
 +
; Reason : Untested code is not production ready by definition. See related rule for a longer explanation.
 +
This also matches an analogous [[#Write_tests_for_your_code|rule]] in the [[#Testing]] section.
 +
 +
== Testing ==
 +
 +
=== Write tests for your code ===
 +
; Importance: HIGH
 +
; Reason : Untested code is dangerous for production. The traditional functional testing performed before a release only covers a portion of the software features: there are many more corner cases, which '''will''' happen in production, to be spotted beforehand. Only carefully written test cases, assisted by coverage statistics and code analysis, can really be of help on this account.
 +
This also matches an analogous [[#Do_not_commit_untested_code|rule]] in the [[#Versioning]] section
 +
 +
=== Make your Unit Tests self-consitent ===
 +
; Importance: HIGH
 +
; Reason : A unit test depending on integration with other systems should probably be called a integration test. When you test a ''unit'', you should test it in isolation. If the unit you are testing requires some ''context'', you have to simulate (i.e. ''mock'') it.
 +
; Suggestion : for Java, use Mockito/Powermock to simulate the interaction of your class/component with an external system.
 +
 +
 +
=== Use appropriate tooling and facilities for testing ===
 +
; Importance: HIGH
 +
; Reason : Tests are automated nowadays, sometimes ''highly'' automated. You can't expect somebody to call your custom <code>main()</code>, as if you do so your test simply won't be executed. Vice versa, if you rely on a standard and proven testing framework, running your test in a broader context will be much easier.
 +
; Suggestion : On Java, use JUnit4 + Mockito/Powermock.

Latest revision as of 13:21, 5 January 2016

Introduction

Rationale

The GreenVulcano code base has been growing over time. While the code has traditionally been "reviewed" by one or two people (our historical "good dictators") before submission to public availability, this is becoming less and less the case due to our increasing core development community. Therefore, some self-discipline is needed in order to improve collaboration - and, sometimes, reduce the frustration due to switching from one style to the other between different source files (and often even within the same file).

The Good Dictators

A dictator is really nothing more than a very senior technical lead. The reason why we use the term dictator is that we as a community take his/her advice very seriously. In fact, one of the few actions having serious consequences for a community member is to contravene an explicit dictator ruling or warning.

Please note:

  • The role of our dictators is to preserve the overall quality of our software over time.
  • Besides spitting out new rules, dictators are usually willing to discuss non-trivial project-related issues. Do not feel afraid to ask if you feel you need some advice.
  • It's a role based on volunteering. Do not voluntarily waste teir time.
  • It's a hard job and takes a lot of time, along with a good deal of experience; do propose yourself as a dictator if you feel you can contribute on this account.

You can contact our dictators at dictators@greenvulcano.com [[1]].

Structure of a Rule

Each rule has, at least:

  • A Name (= Title)
  • An Importance classification:
    • HIGH
    • MEDIUM
    • LOW
    • COSMETIC
  • A Reason explaining why you have to comply

And, optionally:

  • An A.K.A. (also known as) for alternative naming of the same rule, or for easier understanding
  • Suggestions on how to comply with the rule
  • Known Exceptions to that rule. The fact there is a documented exception does not allow you to skip the authorization process for exceptions.

Exceptions to a Rule

Exceptions are generally said to prove the rule. So there must be a rule for exceptions. How exceptions are dealt with depends on the Importance classification of the rule:

  • HIGH: exceptions are not allowed, in any circumstance. Ok, you can ask a dictator, but expect a NO. If, by chance, you get a YES, or even a MAYBE, then probably the rule's importance needs to be degraded to MEDIUM. However, in case an exception ever gets authorized, the exception rule for MEDIUM applies.
  • MEDIUM: exceptions are allowed only if authorized by a dictator, and such authorization must be referenced in the code where the rule is infringed, along with the reason for infringement.
  • LOW: exceptions are allowed only if authorized by the project's technical lead, and such authorization must be referenced in the code where the rule is infringed.
  • COSMETIC: exceptions are not welcome, but tolerated if kept within reason.

Proposing a New Rule

Write to the dictators explaining your proposal, along with the reasons associated to it. It will be much appreciated.

General

This section contain general rules, which apply to all areas of software development. They usually fall into the widely accepted definition of Common Sense.

Contribute to improve software that does not meet the guidelines

A.K.A. 
Do not look away when seeing badly written code.
Importance 
MEDIUM
Reason 
Code tends to live long, especially foundation code. If it's badly written, it has the potential to undermine the overall quality of your software for a very long time (seasoned programmers certainly have their own anecdotes about this). So, whenever you encounter software that doesn't meet one or more of the guidelines in the following document, you definitely should take the time to make it better.

If you don't feel comfortable modifying the code (because it's too tricky to touch it, or because you do not grasp the specific topics addressed by the software), or you just don't have enough time, open a ticket on the bug-tracking system, referencing the exact location of the offending code, along with your observations.


Do not submit copyrighted material without permission

A.K.A. 
Check the license of the software tools and libraries upon which you are basing your software.
Importance 
HIGH
Reason 
Multi-million €/$ lawsuits may arise when powerful companies believe other people have used their own protected assets (such as copyrighted software). It is therefore of vital importance that you make sure you have the right to use somebody else's work for your purposes. OSI-approved licenses ([[2]]) are generally OK - although they also have their subtle differences between each other - while software licensed under different conditions may be very dangerous. Do write a couple of lines to a dictator if you are unsure.

Put a copyright disclaimer and software license in each source file

Importance 
MEDIUM
Reason 
It may happen that a source file is moved somewhere else from where it originally belonged. In such cases, the license originally shipped with the file might not be available anymore. Putting a short disclaimer on top of the file help keeping track of licensing conditions.

Design

This section covers common guidelines concerning the design of our software.

Clearly separate concerns in your design

A.K.A. 
No business and presentation logic mix-up
Importance 
HIGH
Reason 
Whenever possible, our software has to make no assumptions on the context where it runs. As a matter of example: it happened that software originally targeting the desktop needed to be ported to the cloud - it was a bloodshed to separate the actual processing logic from "buttons", "text fields" and "properties".

Never hardcode settings or "magic" values

A.K.A. 
You die if you do that
Importance 
HIGH
Reason 
Your code can (and will) be used in contexts which are totally unknown to you. It will need either to be totally context-independent, or to make all context-related aspects configurable.
Example
A ConfigProperties reader must have a constructor specifying "which" file to read.

Provide a reasonable default for configuration-based software

A.K.A 
Convention Over Configuration
Importance
HIGH
Reason 
We mainly develop middleware. This usually requires a lot of knowledge about the environment the software runs within, which means a lot of configuration. The user may easily feel overwhelmed by having to configure each and every bit. Therefore, when a "default" behavior just makes sense, in absence of a more specific configuration
Example
Besides having a constructor specifying which file to read, a ConfigProperties reader shall also have a constructor omitting such parameter, which shall search for "config.properties" in the classpath or current working directory.

Avoid references to your own workspace files

A.K.A. 
no /home/johndoe/projects on SVN or GIT
Importance 
MEDIUM
Reason 
Every developer has his/her own settings and favourite IDE layouts. If such preferences are placed in a shared, versioned file, a commit war will inevitably start - causing frustration in the entire team/community.
Suggestion 
In order to avoid this, only place common, default settings in the versioned files, and provide a template for unversioned user properties. Then use a smart configuration reader that allows user-specific properties to override the default ones.

Naming and capitalization

Name classes, functions, and variables as the language of your choice indicates

A.K.A. 
Keep consistent with the naming convention of the language you are using
Importance 
MEDIUM
Reason 
Staying consistent with naming helps to improve readability. There's no need for esoteric notations or conventions: all of us are nowadays using IDEs to help us tell static, pointers, constants and the rest apart.
Example 
  (Java)
  public class MyClass {
      
      private static final Object STATIC_FINAL_OBJECT = null;
      private static       Object staticObject;
      
      private              Object instanceObject;
      
      public void myMethod() {
          int methodVariable = 0;
          // something else here
      }
  }
  (Python)
  class MyClass(object):
      def __init__(self):
          self.public_variable = 0
          self._protected_variable = 1
          self.__private_variable = 2
      
      def public_method(self, arg):
          pass
      
      def _protected_method(self, arg):
          pass
      
      def __private_method(self, arg):
          pass

Use the native capitalization convention for the language you are using

Formatting

Indent consistently - four spaces, no tabs

Importance 
MEDIUM
Reason 
Using tabs makes your code render differently on different devices (e.g. some may have tab stops set at 8 spaces, while yours might be 4 spaces).

Remove unused code

Importance 
LOW
Reason 
Unused / commented code is confusing and complicates reading. Versioning tools are great at restoring deleted code, should you need it back. Unused code also includes unused import/include directives.

Versioning

Do not commit broken code

A.K.A. 
Do not break the software build.
Importance 
HIGH
Reason 
When you commit broken code (i.e. code that does not work properly, or - even worse - doesn't compile) you are negatively affecting the entire team's productivity. People might be blocked waiting for your fix in order to test their own work, and centralized tools such as code analysis, continuous build and integration testing will be unreliable for the hours or days to come.


Do not commit untested code

Importance 
MEDIUM
Reason 
Untested code is not production ready by definition. See related rule for a longer explanation.

This also matches an analogous rule in the #Testing section.

Testing

Write tests for your code

Importance
HIGH
Reason 
Untested code is dangerous for production. The traditional functional testing performed before a release only covers a portion of the software features: there are many more corner cases, which will happen in production, to be spotted beforehand. Only carefully written test cases, assisted by coverage statistics and code analysis, can really be of help on this account.

This also matches an analogous rule in the #Versioning section

Make your Unit Tests self-consitent

Importance
HIGH
Reason 
A unit test depending on integration with other systems should probably be called a integration test. When you test a unit, you should test it in isolation. If the unit you are testing requires some context, you have to simulate (i.e. mock) it.
Suggestion 
for Java, use Mockito/Powermock to simulate the interaction of your class/component with an external system.


Use appropriate tooling and facilities for testing

Importance
HIGH
Reason 
Tests are automated nowadays, sometimes highly automated. You can't expect somebody to call your custom main(), as if you do so your test simply won't be executed. Vice versa, if you rely on a standard and proven testing framework, running your test in a broader context will be much easier.
Suggestion 
On Java, use JUnit4 + Mockito/Powermock.