User:Mvglasow/Git guidelines (draft)
This is a proposal for new Git guidelines.
The current guidelines were drawn up before the switch to Git (prior to that, we used SVN for a long time). However, a few things have changed since then: we have since switched to Git, we now have an automated test suite, and maybe some of the existing guidelines may need some tweaking or clarification.
Most of the proposed guidelines below came out of a discussion between sleske and me, as well as the existing SVN guidelines.
Add, Fix and Refactor
We distinguish three types of commits:
- Add: these commits introduce new functionality.
- Fix: these commits fix a bug in an existing feature. They do not introduce new functionality.
- Refactor: these commits are for code changes that do not change the behavior of Navit, such as changing internal data structures, renaming identifiers, adding Doxygen comments and similar.
Push to trunk, not to master
The trunk branch is where development happens, while master is the "stable" branch. Pushing to trunk will trigger CircleCI and, if all tests pass successfully, trunk will be merged into master.
Commit per feature
When committing, try to have one commit per feature (or per meaningful part of a larger feature). Avoid putting multiple, independent changes into one commit. Instead, commit one feature before you start work on the next one. Consider creating different branches for different features.
Also, try to avoid mixing new functionality, fixes and/or refactoring in one commit. Instead, commit each type of change separately. New features often require a certain amount of refactoring – do that first, then test and commit the refactored code. After that, implement, test and commit your new feature separately.
Commit working code
The goal is to always have working code. At least make sure each commit leaves the repository in a compilable state. if you can run tests to verify that it works properly, that's even better.
If you're working on your own fork of Navit, you can obviously commit what you like, but please do not merge "broken" commits back into the main repository. Instead, rebase your commits before you merge them.
We highly recommend that you use CircleCI to test your commits. There are two easy ways to do that:
- Use merge requests. Every merge request to the main repository is passed through CircleCI and you will see in the merge request on Github whether the tests have passed successfully.
- Set up CircleCI to track your own repository. This can be done in a few steps, and CircleCI tests will run on anything you push to your repository.
If for some reason you can't or don't want to use CircleCI, at least build the code once (preferably for multiple platforms) before you push it to the main repo.
Commits are easier to analyze and troubleshoot if they are smaller. Smaller commits are also much easier to rebase (see next section). Therefore, try to make small changes, test them and commit them right away.
Obviously, there is no hard-and-fast rule for commit size or granularity here. Examples of changes which should be committed separately from other changes are:
- Undoing (completely or partially) code changes introduced in an earlier commit
- Fixing a bug introduced in an earlier commit
This is by no means an exhaustive list – use common sense.
Once you are ready to push everything to trunk, rebase. By rebasing, you are essentially taking your commits and applying them to a different branch. You can also change the order of your commits, squash multiple commits together, amend commits or change commit messages. Read up on rebasing if you haven't used this feature before – it takes a bit of learning, but it's a powerful feature and can help you keep your commit history much more legible.
Start by creating a new branch forked from your feature branch. (Not required, but this way you still have your old development history around in case something goes wrong.) Pull the latest revision of trunk from the main repo. Then do an interactive rebase on top of trunk and rearrange commits as needed. If you have commits which undo code changes introduced in an earlier commit, or which fix bugs introduced in an earlier commit, squash them into that commit. (This will work much better if you have followed the previous rule.) After testing the commits to ensure each satisfies the #Commit working code rule, you can merge it into trunk and push it.
TODO: can we tell CircleCI to run on every single pushed commit, not just the last one?
Don't rewrite history
Once you've pushed to the official Navit repository, do not amend, rebase or in any other way modify the commit history, as this may break things for other people. If you need to force the push, it's a sure sign you shouldn't be pushing this. If you've accidentally pushed buggy code to the main repository, add another commit on top that fixes it.
Of course, this goes only for changes to the official repository – feel free to do as you please in your own fork, in a branch you've never pushed to the official repository, or with commits you haven't pushed yet.
'Core' component changes
Do not modify a 'core' component without discussing it first with the project leads (see team).
Core components include data structures (generally, everything exported in a header file), configuration handling. If you are unsure, just ask.
Format of the commit log
TODO: is this still valid?
Since we are too lazy to maintain a Changelog, we have a script which parses the commit logs and generate a Changelog for us.
We have agreed about using the following syntax :
<Action>:<component>:<log message>[|Optional comments]
Fix:Core:Fixed nasty bug in ticket #134 Fix:GTK:Fixed nasty bug about destination button|Thanks someguy for the patch!
Action can be something like:
- Fix (bug fix)
- Add (new feature)
Patchshould no longer be necessary
- Refactor (does not change behavior of the program)
- Merge (?)
It allows the changes to be sorted by categories. See #Add, Fix and Refactor for an explanation.
Component is the component field in the trac. The most common are :
The comment part is optional. Useful for when applying a patch for example, and giving credits. The part after | will not appear in the wiki.
About the log message, it's up to you :)