Project

General

Profile

Contributing Code » History » Version 6

Simon Spannagel, 06/22/2012 12:14 PM

1 1 Simon Spannagel
h1. Contributing Code
2 1 Simon Spannagel
3 1 Simon Spannagel
h2. Fork vs. patch
4 1 Simon Spannagel
5 1 Simon Spannagel
The favored way of contributing code to darktable is through github forks. We will continue to deal with patches in our bug tracker or on the mailing list, but github fork are much easier for us to deal with. They allow per-line comment, they are easy to merge, we can more easily look at progress.
6 1 Simon Spannagel
7 1 Simon Spannagel
And more importantly, when you fork on github and have a public branch, we see it immediately through the github repo network (see https://github.com/darktable-org/darktable/network). This means that we could comment on a feature before you invest too much time in something we don't like. But please keep in mind: it's always better to discuss your projects on the mailing list or IRC beforehand...
8 1 Simon Spannagel
9 1 Simon Spannagel
So yes, fork a lot, it's a great thing, even for trivial patches.
10 1 Simon Spannagel
11 1 Simon Spannagel
12 1 Simon Spannagel
h2. Naming branches
13 1 Simon Spannagel
14 1 Simon Spannagel
If you work on a large feature (any feature that you expect to generate multiple commits) please give it a proper name related to that feature (i.e avoid naming the branch after your own name). It will make things easier once you are done and start working on something else, and it's easier for us when browsing through branches to figure out what you are working on. If you are working on something related to an issue in our tracker, using a feature request or bug issue number as branch name is a good idea, too. The other way round: it might be worth to add a ticket to the tracker and set the status to "In progress" so people can easily see you are currently working on this.
15 1 Simon Spannagel
16 1 Simon Spannagel
If you just want to do a quick fix it's ok to work on (your fork's version of) master. If you have multiple trivial patches, stack them on master, that's fine. Just make sure that you do a pull request often and re-merge the main master regularly. You can also create a branch named after a bug of course, but for trivial fixes, working on master is less of a problem for us.
17 1 Simon Spannagel
18 1 Simon Spannagel
Last, if you have finished working on a feature, have create a pull request and start working on something else, please recreate a new branch based on the main master branch, and do not commit new things on top of your first feature branch (that is: assuming both features are independant of course). This will allow us to merge your latest feature independently if there is a problem with the first one that has to be fixed before merging back. It will make things easier for you if we are not happy with your feature and you have to add commits after the pull requests, and make things easier overall.
19 1 Simon Spannagel
20 1 Simon Spannagel
21 1 Simon Spannagel
h2. Creating a Pull Request
22 1 Simon Spannagel
23 1 Simon Spannagel
Don't create a pull request until you are ready to merge your work. A pull request is a call for us to go through your work and then merge it back in the main darktable repository. So you should only create your pull request once you think your feature is ready.
24 1 Simon Spannagel
25 1 Simon Spannagel
This also means that you can post a "request for testing/review" on the mailing list for any work in progress, you don't need a pull request for that...
26 1 Simon Spannagel
27 1 Simon Spannagel
We might need some time to deal with your pull requests so don't be suprised if there is no news for a week or two... We do our best but we are all volunteers, too. On a related note, the bigger the change, the more time it will need for review. So don't expect new modules to be handled as fast as one-line changes, especially if the changes contain new image operators changing our processing pipeline.
28 1 Simon Spannagel
29 1 Simon Spannagel
Pull requests are attached to branches, not commits. This means that if you push some commits on a branch that has a pull request pending, the pull request will move with the branch head. This allows you to continue to work on a branch to fix whatever issue we have with it, but it also means that once you've done a pull request you should not add unrelated work on top of it. Use a new branch for that.
30 1 Simon Spannagel
31 1 Simon Spannagel
Since we need time to review a pull request, please try to keep it up to date by regularly merging with our master branch (a good rule of thumb is to re-merge master at least whenever auto-merge won't work, which can be seen at the bottom of the pull request). This will save us some time and remind us that you are waiting on us and that we should do something. Having your contribution based on the latest master branch prevents us from having to solve merge conflicts once your pull request is reviewed.
32 1 Simon Spannagel
33 2 Simon Spannagel
We will usually not pull unless there is a pull request. I.e. don't assume your single change will be noticed. It usually will be but unless you do a PR we will assume you have a good reason for not wanting it merged.
34 2 Simon Spannagel
35 2 Simon Spannagel
We might pull if we think your idea is very important and we are ready to work on it to complete it (or you are unresponsive to comment and we are ready to take matter in our own hand) More commonly we might pull any quick fixes, especially if we are about to release, but these are special cases. Our rule of thumb is that your branch is yours and we will not pull until we are invited to.
36 1 Simon Spannagel
37 1 Simon Spannagel
h2. Assigning and Merging Pull Requests
38 1 Simon Spannagel
39 1 Simon Spannagel
(this paragraph is more for devs than for external contributors)
40 1 Simon Spannagel
41 1 Simon Spannagel
Pull requests can be assigned to a dev. Assigning it means that the PR is not for grab anymore, it is an "exclusive lock" on that request. In other word, the assigned dev is the one responsible for merging and other devs shouldn't merge without asking him first. As a consequence
42 1 Simon Spannagel
* Don't assign PR to other devs unless you've asked first
43 1 Simon Spannagel
* Devs should feel completely free to de-assign themselves if they don't want/don't have the time to deal with a PR.
44 1 Simon Spannagel
* Unassigned pull requests are up for grab. Any dev can merge them.
45 1 Simon Spannagel
46 1 Simon Spannagel
This does not mean that other devs can't comment on an assigned request. This is more of an "I am working on this area I want to look into it" kind of lock. Any dev should feel free to comment on any request or ask for comments from any other dev (by including the name in the comment, there is a special wiki markup in comments for that).
47 3 Jérémy Rosen
48 5 Simon Spannagel
h2. Commit messages
49 5 Simon Spannagel
50 6 Simon Spannagel
When committing think of clear commit messages that describe your changes for that specific commit. This will make it easier for others to follow up with your work and find problems if they occur. The commit messages are pulled with their commits to our main repository once your pull request is merged. Since our Redmine has access to this it processes the commit messages. So you are able to link to issues within Redmine from your commit message - or even close a bug without visiting the bug tracker. Just type "this fixes #1234" to close or just "#1234" just to mention the issue somewhere in your commit message. Have a look at the [[Bug_Workflow#Redmine-git-integration]], section "Redmine git integration" for further information.
51 5 Simon Spannagel
52 3 Jérémy Rosen
h1. TODO
53 3 Jérémy Rosen
54 4 Anonymous
* add a note about working on a clean copy to avoid pulling in other people's changes
Go to top