You are here: Noosfero>Dev>PatchGuidelines (19 Oct 2011, AurelioAHeckert) EditAttach

Patch Guidelines

Get noosfero source code and setup to run the tests

See Getting started with Noosfero development

Create a new branch for your patch

git checkout -b my_hew_branch master

You can put the name you like in your branch.

Write automated tests for your code

Noosfero extensively uses automated tests to make sure we don't break anything when we need to make changes to the system. For that reason, we ask you to always write automated tests together with your patches:

  • if your patch fixes a bug, make sure you have at least one automated test that fails without your fix and passes with it; you can see examples of automated tests in the test/ directory in Noosfero source code; tests for model classes are in test/unit, tests for controllers are in test/functional, and integration tests are in test/integration. Please also consider writing automated acceptance tests (see below) if the bug or its fix has user interaction implications
  • if your patch implements a new feature, make sure you write a cucumber acceptance test that explains how the user is supposed to use that feature ("the user clicks on 'Control panel', then selects option 'Content Management', then selects 'new article' ..."). You can find examples under the features/ directory.

Please keep in mind that the chances of getting a new feature in Noosfero without proper automated tests is very low.

Make sure the existing tests pass with your changes in

One of the reason why we have automated tests is to be relaxed while changing the code knowing that if we break anything the tests will tell us that before our new code goes into a release. So, make sure to run all existing tests (just run `rake` in Noosfero sourcode top directory).

Commit your changes

  1. Add the files that you changed to the commit with: git add path/to/changed/file.rb (repeat this step for every file you changed want to include in the patch)
  2. And then commit your changes: git commit (you should mention an action item, see below)
  3. Write an understandable summary for your patch, describing what your patch does, not what was the problem before it. For example, instead of "Error when doing this and that", write "Fixing error when doing this and that"
    • if you can't write your thoughts down in understandable English, ask for help. smile

Assign an action item to your patch

Every patch must be related to an existing action item, so we can track what was needed to fix a bug or implement a feature. To do this, you should mention an action item in some part of the commit message. You can do that in the end of the message, like the following examples:

  Fixes problem when X happens in Y
  
  (!ActionItem1234)
  Implement Z feature

  This implies the following assumptions:...

  (!ActionItem9999)

If you think it's hard to remember this, you can paste the following code in your .git/hooks/commit-msg:

if ! grep -qs -e '^ActionItem[0-9]\+:' "$1"; then
        echo "You must assign your commit to an existing action item!"
        echo "Please refer to http://noosfero.org/Development/PatchGuidelines"
        exit 1
fi

After pasting, make sure that file is executable (so git will run it before letting you commit) with:

chmod +x .git/hooks/commit-msg

Generate patch — The old fashion way

After you commited your change in your private repository, you may to send a patch to the development mailing list. To do that run the following command: git format-patch master..my_branch — Git will leave a file with a name like 0001-my-commit-message.patch in your current directory, that's the patch you should send to the mailing list.

The ways to send your patch by e-mail, in order of preference:

  1. Using git send-email
  2. attach the patch file to the e-mail
  3. attach the patch file to the corresponding action item, and send the URL.
  4. attach the patch file to some random place in the internet, and send the URL.

Please try the ones in the top before using the ones in the bottom:

Request a Merge on Gitorious — The best way

If you forked Noosfero at Gitorious, you can push your commits to your Gitorious repository and make a merge request by the web site interface. Just visit your repository page and click on "Request merge".

Request Merge

Patch review checklist

This following checklist will be followed for reviewing your patch, so it would be interesting if you do it yourself too before submitting the patch. It's not definitive, and it will certainly grow with time.

  • general aspects
    • follow the indentation style already used (2 spaces)
    • the patch must not contain trailing whitespace (git show or git diff will highlight them if color is enabled)
    • the patch must identify an action item in its log message
    • be consistent. If you do something in some way in one place, do it the same way in all places.
  • migrations
    • when a new column is added and it has a default value, make sure the existing records are updated to have this default value just after the column is added (e.g. execute("update TABLE set NEWCOLUM = DEFAULTVALUE")). That seems redundant, but not all RDBMS systems apply the default value of a new column to the existing records in the table.
  • models: all application logic should be in the models. Data validations, restrictions, operations that query the database directly or modifies anything must be in the corresponding model.
  • controllers: make sure controllers only do the work needed in order to implement user interaction. Let all validations and other types of logic be implemented in a model class.
  • tests: avoid writing tests that take too long to run; avoid hitting the database (i.e. saving objects to it) unless absolutely necessary.

Special Patching Methods


Comments

é simples não, esta em inglês... Cadê a versão pt_BR?

-- ValessioBrito - 05 May 2011

Who can't understand a simple technical english may not be good enough to make a patch. Right?

-- AurelioAHeckert - 10 May 2011


Add comment
You need to login to be able to comment.
 

Topic revision: r14 - 19 Oct 2011 - 22:54:48 - AurelioAHeckert
 
Translations: English
Global Search:
   
ActionItem Search:

Copyright © 2007-2013 by the Noosfero contributors
Colivre - Cooperativa de Tecnologias Livres