Guidelines
This is not meant to be a strict set of rules, but merely a emphasized suggestion, a SVN etiquette if you might. This page is for both developers with commit rights and developers without.
Intro
There are many more submitters than reviewers, therefore the only way we can sanely handle patches/commits is not reviewing patches that don't follow the guidelines. Some developers with extra time may review such patches, but it's just better for everyone if the patches just follow the guidelines. The optimal reviewing scenario is: open an email containing a patch, read a nice summary about what the patch does, review the patch to see it's correct, apply, compile, test and if all is well, *copy* the commit message from the email containing the patch. Obviously, if the patch fails to comply to any of the following guidelines, you are encouraged to deny it and request for another patch.
Coding style
The official E coding conventions are at: ECoding. Patches should *NOT* introduce coding conventions violations. Furthermore, patches should not include whitespace errors (trailing whites, tabs).
Valgrind and Compilation warnings
The patch should not introduce any new warnings with default configure flags and "-Wall -Wextra". (Note: TBD need to add the whole set of warnings here and to the READMEs) The patch also should not introduce new Valgrind complaints, especially: invalid reads/writes and memory leaks. Even worse than compilation warnings, is force-silencing the warnings flags, that is, just employing tricks to fool the compiler so it won't complain instead of just fixing the actual issue.
Documentation
All public API functions should be documented using doxygen. You can take a look at Eina/Eet? for examples, or just read the doxygen docs. Internal functions and code should also be documented (as much as needed), but this is only a recommendation. Using doxygen for internal functions is recommended, you just need to use '@internal' to mark it as such.
SVN Commit Log
Good SVN log messages are not hard to write, the idea is to explain the change in a couple of lines in a way that other people (even after long periods of time) will be able to understand.
Preferred log message format:
Lib/app module: Short summary. (len <= 72) Long summary More long summary
For example:
Elementary calendar: Take Weekday abbreviated names from locale. This information is already retrievable from the locale, no need to use strings and translate them.
ChangeLog?
The changelog file should be updated on every change worth mentioning, this is really a matter of opinion, so in case of doubt, be on the safe side and add an entry. For example: you shouldn't add "fix a typo in a comment" to the changelog, but you should add "rewrote the entry widget".
The format of ChangeLog? file is fairly simple and easy to understand, just follow the same format. New changes are listed at the bottom. Just don't forget to start your commit by either : Fix, Add or Remove, Rewrote (This help during release cycle to be sure that every thing is properly backported).
Updating the changelog is very important because it's an easy and fast way to know what was changed between two points of time, both for users and developers.
More patch guidelines
- Patches should not include unrelated changes (fixing both a and b in the same patch, if they can be split, they should be split).
- Patches should not include formatting and logic changes in the same patch. For example, fixing indentation and changing a '+' to a '-' in the same patch is prohibited.
- Patches should be as small as possible and split among several patch files. It's terribly hard to review huge patches that change thousands of lines.
Sending patches to the developers mailing list
- Patches should follow all of the guidelines in this document.
- E-mail should include:
- The patch (as an attachment, not in the body)
- An svn log the reviewer can just copy and paste.
- Summary of the patch (what it does, what was the issue and etc.).
- A good and descriptive subject line.
Notes for git (git-svn)
Sending git patches is allowed and welcomed. It means we get nice patches with a commit message already attached to each. Gustavo wrote a nice tool for applying git-svn patches when they include binary data changes, moving files, adding files and etc (obviously people who have git-svn can just use that). Feel free to use it: http://barbieri-playground.googlecode.com/svn/python/svn-git-am.py
More simple git patches can just be applied using "patch".
Important note (troll) from Tom: git lets you do a lot with little effort. It's also very convenient for creating small and compact patches, sending email integration is awesome, and I advise everyone to use it.
Reviewing patches
Feel free to review patches, even if you don't have commit access or you are not a maintainer of the specific piece code. You don't have to be an E-Jedi (master?) to know when a patch includes whitespace errors/compilation warnings or even logic failures. Your review of the patch will speed things up, save time for other people and improve the code quality of E.
Case specific comments
Developers without commit rights
Submit a new bug in trac and send a mail to the mailing list containing a brief explanation and a link to the bug report. If you receive no feedback in the course of 15 days try insisting with another mailing list message.
Warning: mail list strips binary attachments and some clients fail to recognize *.patch and *.diff properly. This is common with GMail and other webmail. Before sending patches, ensure you have the following line in your /etc/mime.types:
text/x-diff diff patch
Patch file format
Now that we're using SVN, patches are already in "unified" format, so nothing is really needed. Just use:
svn diff -x -up PATH > 0001-my-first-mod.patch
Even more helpful is you to review the patch contents and check if there is no redundant stuff or useless white space changes, You can also provide a diffstat and add comments to the patch (before the patch itself). The process summary is:
svn diff -x -up > 0001-my-first-mod.diff less 0001-my-first-mod.diff # review!!! $EDITOR 0001-my-first-mod.message # write your description cat 0001-my-first-mod.message > 0001-my-first-mod.patch diffstat 0001-my-first-mod.diff >> 0001-my-first-mod.patch cat 0001-my-first-mod.diff >> 0001-my-first-mod.patch mail 0001-my-first-mod.patch
It's good to keep 0001-my-first-mod.message as a subject line up to 72 chars, then a blank line and then the description body, breaking lines at 72chars (good for mail)
Note: The use of git-svn is allowed (read in the section above).
Warning: please check ECoding to know exact coding style before sending patches! It's quite unusual coding style and most people will send patches that will mess it up if our coding style is not followed.
New developers WITH commit rights
Depending on the type of commit you intend to make there are two paths to follow. If you feel confident about your patch, be it because you are a confident person or because the patch fixes a trivial problem, go ahead with your commit. But be warned, developers constantly breaking the build will have their commit rights removed.
If you don't feel confident, probably because your patch implements new functionality or API changes, simply follow the regular path. Submit a new bug in trac and send a mail to the mailing list containing a brief explanation and a link to the bug report. If you receive no feedback in the course of 15 days you are free to commit it without fear.
You have no exempts, you also have to follow the above guidelines completely.
Warning: please check ECoding to know exact coding style before sending patches! It's quite unusual coding style and most people will send patches that will mess it up if our coding style is not followed.
Add yourself to AUTHORS and doc/{module}.dox.in
We keep no per-file authorship. Instead we use the AUTHORS file in every project.
If your patch contains meaningful code (ie: not just coding style or formatting cleanups), please include yourself to AUTHORS file! If you forget to do so, you can always request it later, but it is better to do it with the patch you're about to send.
To apply AUTHORS changes to the documentation, please include yourself to doc/{module}.dox.in file as well.
