Политика внесения изменений
Далее описаны главные правила для создания и публикации изменений в любые репозитории Qt. Как обычно в Qt, ни одно из этих правил не является абсолютно верным, но вы будете обьектом насмешек если нарушите их без обьективной причины.
- Если вы добавляете новую функцию/класс:
- Убедитесь в том, что все задокументировано правильно.
- Следуйте API_Design_Principles. Обсудите и сверьте имена функций/классов с другими Qt разработчиками (conduct API reviews).
- All code should follow the Coding_Conventions.
- For GUI code, follow the style guide [techbase.kde.org] (yes, that’s KDE’s), the Qt Creator translation hints [developer.qt.nokia.com] and avoid internationalization mistakes [techbase.kde.org].
- Ensure your change compiles and works on all platforms.
- Verify that there are no regressions in the unit tests (see Qt_Autotests_Environment for more info).
- Write new unit tests for the bugs you fixed or functionality you added.
- Make your new/changed code follow the Qt_Coding_Style.
- Produce commits which facilitate reviews and contribute to a useful history which can be read, searched, annotated and cherry-picked. Here are some hints:
- Make minimal and atomic commits. That means that each commit should contain exactly one self-contained change – don’t mix unrelated changes, and don’t create inconsistent states. Never “hide” unrelated fixes in bigger commits. Make coding style fixes only in exactly the lines which contain the functional changes, and comment fixes only when they relate to the change – the rest is for a separate commit.
- Write descriptive commit messages. Make them self-contained, so people don’t have to research the historical context to make sense of them. Conversely, don’t put unnecessary trivia into them. Tell why you changed something unless it is completely self-evident. Use the Qt commit template [qt.gitorious.org]: Follow the summary + description message style and use pseudo-headers to reference JIRA issues, reviewers, etc. and consider the generic Git commit message guidelines [tbaggery.com].
- Commit often. Use
git rebase -iextensively to get your history into shape.
- Avoid unnecessary merges. Use
git pull —rebaseunless you have an unpushed “proper” merge.
- Get a review from somebody who knows the code before you publish. If there is no candidate yet, introduce somebody to the code. Note that pushing to your private clone does not count as publishing and is a perfectly valid way to solicit a review or make a backup of a work in progress.
- Don’t commit anything you don’t understand. “Somehow it suddenly works” is not acceptable. Reverting when a proper fix would be possible is admitting defeat. ;)
- Review your changes before you push.
git log —stat,
git log -pand
gitkare your friends.
- Don’t fight the git pre-receive hooks. See below.
- And most importantly: use your brain before hitting
The pre-receive Hooks
Our internal git repositories have various pre-receive hooks installed which ensure some minimal quality standards and legal safety. The sanitizer script (without the privacy check) is available from the devscripts repository [qt.gitorious.org]. The checks are:
- The privacy check. It tries to ensure that no real names are published without explicit consent. A side effect is that it will complain about invalid committer information, which is Good ™. Valid addresses from external contributors are added to a whitelist; that process is semi-automated.
- The line ending check. We are developing in a heterogeneous environment with both Unix and Windows machines. Therefore it is imperative to have all files in the repository in the canonical LF-only format. Windows users need to set the git option
trueto automatically get CRLF line endings which are suitable for the native tools.
- Conflict marker check. Unless you are adding a text file which contains the patterns on purpose, you most probably botched a merge/rebase.
- Generated file check. Don’t commit generated files – they bloat the repository and create spurious changes. If a file is generated but you don’t know how to automate the build process, ask an expert.
- Check for project/config files of alien build systems. As we are using qmake, these will in most cases fall under the “generated file” rule anyway.
- Big file addition check. Think three times whether you really need that huge media file, test binary or screenshot. As we are using git, such mistakes are not correctable – once you added a huge file, everyone who clones the repository will have to pay for it with network traffic and disk space for all times. Source code files are exempt from the check at all; for the rest it is a bit paranoid (it will complain about anything bigger than 50 KiB or an instantaneous file growth over 150% if the file is bigger than 20 KiB).
- Huge file addition check. If you trigger that one, you almost certainly did something wrong.
- Work In Progress push check. This is meant to avoid that you accidentally push something you marked for later cleanup. It checks for the strings “WIP” and “***” in the summary line and for an empty Reviewed-by line. This check is disabled in the local post-commit hook for obvious reasons.
- Check for mixing whitespace and non-whitespace changes. A commit may contain only whitespace changes or “proper” changes (with whitespace cleanups only on already changed lines, and, as an exception, additions and removals of empty lines). This helps to ensure the usefulness of
git blame(note that a clean history is better than using
git blame -w, as the latter will also hide significant whitespace changes). (currently advisory only)
- Some simple coding style checks. (currently advisory only)
The checks can be overridden on a case by case basis when needed. Don’t even think about overriding any of them unless you have a really good reason. “I need to get this done now” is not a valid reason. Neither is “I cannot be bothered to understand what it wants from me”. If there is any doubt, discuss alternatives with somebody appropriate.
Each error/warning message mentions the key needed to override it. Note that there is only one key per entire category, so make sure to check all reports before you apply the override.
The recommended override command is printed by the hook itself. Don’t export the environment variable in the current shell, as you will inevitably forget to unset it again and thus disarm the hooks semi-permanently.
For the overrides to work, you need to tell ssh to forward the GIT_PUSH environment variable to the git server. If you don’t have an ssh config file yet, create one. It is probably wise to put a
Host scm.dev* line in front of the
SendEnv line to restrict the scope of the setting. Under Windows, the file will be usually located in
“C:\Documents and Settings\<user>\.ssh\config”, though apparently it is possible to set HOME like under Unix to change ssh’s behavior.
It is possible to run most of the checks locally before attempting to push – this is especially useful for the checks which are currently advisory only. See the
devtools/shell/git_post_commit_hook script (it contains installation instructions).