Commit Policy: Difference between revisions

From Qt Wiki
Jump to navigation Jump to search
m (wording)
(reshuffle, and link to review policy)
Line 6: Line 6:
## Ensure everything is documented properly.
## Ensure everything is documented properly.
## Follow the [[API Design Principles]]. Discuss and verify the names of functions/classes with other Qt developers (conduct API reviews).
## Follow the [[API Design Principles]]. Discuss and verify the names of functions/classes with other Qt developers (conduct API reviews).
# All code should follow the [[Coding Conventions]].
# Make the new/changed code follow the [[Qt_Coding_Style|coding style]] and [[Coding_Conventions|coding conventions]].
# For GUI code, follow the [http://techbase.kde.org/Projects/Usability/HIG style guide] , the [[Qt_Creator_Translation_Page|Qt Creator translation hints]] and avoid [http://techbase.kde.org/Development/Tutorials/Localization/i18n_Mistakes internationalization mistakes].
# For GUI code, follow the [http://techbase.kde.org/Projects/Usability/HIG style guide] , the [[Qt_Creator_Translation_Page|Qt Creator translation hints]] and avoid [http://techbase.kde.org/Development/Tutorials/Localization/i18n_Mistakes internationalization mistakes].
# Ensure your change compiles and works on all platforms, also when Qt is [[Qt In Namespace|built in a namespace]]. The CI system rejects broken changes.
# Ensure your change compiles and works on all platforms, also when Qt is [[Qt In Namespace|built in a namespace]]. The CI system rejects broken changes.
# Verify that there are no regressions in the unit tests (see [[Public Autotest Infrastructure]] for more info).
# Verify that there are no regressions in the unit tests (see [[Public Autotest Infrastructure]] for more info).
# Write new unit tests for the bugs you fixed or functionality you added.
# Write new unit tests for the bugs you fixed or functionality you added.
# Make your new/changed code follow the [[Qt_Coding_Style | coding style]].
# Do not commit anything you do not understand. "Somehow it suddenly works" is not acceptable. Reverting when a proper fix would be possible is admitting defeat. ;)
# Produce commits which facilitate reviews and contribute to a useful history which can be read, searched, annotated and cherry-picked. In particular:
# Produce commits which facilitate reviews and contribute to a useful history which can be read, searched, annotated and cherry-picked. In particular:
## Make atomic commits. That means that each commit should contain exactly one self-contained change - do not mix unrelated changes, and do not 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.
## Make atomic commits. That means that each commit should contain exactly one self-contained change - do not mix unrelated changes, and do not 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.
Line 17: Line 17:
## Commit often. Use <tt>git gui</tt> and <tt>git rebase -i</tt> extensively to get your unpublished history into shape. Note that pushing to your private clone does ''not'' count as publishing and is a perfectly valid way to solicit an early review or to make a backup of a work in progress. Further reading: [http://sandofsky.com/blog/git-workflow.html Understanding the Git Workflow]
## Commit often. Use <tt>git gui</tt> and <tt>git rebase -i</tt> extensively to get your unpublished history into shape. Note that pushing to your private clone does ''not'' count as publishing and is a perfectly valid way to solicit an early review or to make a backup of a work in progress. Further reading: [http://sandofsky.com/blog/git-workflow.html Understanding the Git Workflow]
## Avoid unnecessary merges. Use <tt>git pull —rebase</tt> unless you have an unpushed "proper" merge. Our Gerrit permits only trusted Merge Masters to push merge commits.
## Avoid unnecessary merges. Use <tt>git pull —rebase</tt> unless you have an unpushed "proper" merge. Our Gerrit permits only trusted Merge Masters to push merge commits.
# Do not commit anything you do not 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 to Gerrit. <tt>git log [--stat] [--summary] [-p] @{u}..</tt> and <tt>gitk</tt> are your friends.
# Review your changes before you push to Gerrit. <tt>git log [--stat] [--summary] [-p] @{u}..</tt> and <tt>gitk</tt> are your friends.
# Make sure to follow the [[Branch Guidelines]]. Submit against the lowest applicable branch from which a release is still planned. Cherry-picks ("backports") are strongly discouraged (use the <tt>-x</tt> option if you still do), while forward-merging to more recent branches happens "automatically" on a regular basis.
# Make sure to follow the [[Branch Guidelines]]. Submit against the lowest applicable branch from which a release is still planned. Cherry-picks ("backports") are strongly discouraged (use the <tt>-x</tt> option if you still do), while forward-merging to more recent branches happens "automatically" on a regular basis.
# Peer review is '''strongly''' encouraged. Do not approve your own changes. Discuss objections.
#* If there is no candidate for a review yet, introduce somebody to the code.
#* Maintainer privilege: A maintainer ''may'' approve his own change to the code he maintains if
#*# at least one review (+1) from somebody else is present, and
#*# nobody else who could approve (+2) the change can be produced within reasonable time
# Do not ignore/fight the [[Early Warning System]]. Justify each override.
# And most importantly: use your brain.
# And most importantly: use your brain.
Next, follow the [[Review Policy]].


== Change Log ==
== Change Log ==


If the change is significant and affects many users, compatibility, or is a noteworthy feature, you must add a ChangeLog tag. Conversely, do not add ChangeLog entries that are not relevant to users.
If the change is significant and affects many users, compatibility, or is a noteworthy feature, you must add a ChangeLog tag. Conversely, do not add ChangeLog entries that are not relevant to users.
* A [ChangeLog] entry can be multiple lines long and ends with an empty newline.
* A [ChangeLog] entry can be multiple lines long and ends with an empty line.
* Try to integrate the changelog entry into the surrounding commit message to avoid redundancy.
* Try to integrate the changelog entry into the surrounding commit message to avoid redundancy.
* If the Git repository contains multiple modules, use the module name to indicate the area of the change e.g. [QtCore].
* Iff the Git repository contains multiple modules, use the module name to indicate the area of the change e.g. [QtCore].
* Optionally specify a class or subtopic [QtNetwork][QSslSocket]
* Optionally specify a class or subtopic [QtNetwork][QSslSocket]
* Other common tags are:
* Other common tags are:
Line 42: Line 37:
** [OS X]
** [OS X]
** [Linux/XCB]
** [Linux/XCB]
* In summary the entry should look like this:<pre>[ChangeLog][module][class/topic] description of the really important&#10;change that was just made (on several lines).</pre>
* In summary, the entry should look like this:<pre>[ChangeLog][module][class/topic] description of the really important&#10;change that was just made (on several lines).</pre>
** The description should use either simple past ("Fixed …") or be explicit about referring to the current state ("… does now …").
** The description should use either simple past ("Fixed …") or be explicit about referring to the current state ("… does now …").
** Make sure the entry is reasonably self-contained. If you fail to formulate a meaningful description, it's probably not useful information to start with.
** Make sure the entry is reasonably self-contained. If you fail to formulate a meaningful description, it's probably not useful information to start with.

Revision as of 08:44, 13 July 2016


These are the general rules for creating and pushing commits to any shared Qt repositories. As usual at Qt, none of these rules is set in stone, but you will be subject of public reprimand if you violate them without good reason.

  1. If you add new functions/classes:
    1. Ensure everything is documented properly.
    2. Follow the API Design Principles. Discuss and verify the names of functions/classes with other Qt developers (conduct API reviews).
  2. Make the new/changed code follow the coding style and coding conventions.
  3. For GUI code, follow the style guide , the Qt Creator translation hints and avoid internationalization mistakes.
  4. Ensure your change compiles and works on all platforms, also when Qt is built in a namespace. The CI system rejects broken changes.
  5. Verify that there are no regressions in the unit tests (see Public Autotest Infrastructure for more info).
  6. Write new unit tests for the bugs you fixed or functionality you added.
  7. Do not commit anything you do not understand. "Somehow it suddenly works" is not acceptable. Reverting when a proper fix would be possible is admitting defeat. ;)
  8. Produce commits which facilitate reviews and contribute to a useful history which can be read, searched, annotated and cherry-picked. In particular:
    1. Make atomic commits. That means that each commit should contain exactly one self-contained change - do not mix unrelated changes, and do not 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.
    2. Write descriptive commit messages. Make them self-contained, so people do not have to research the historical context to make sense of them. Conversely, do not put unnecessary trivia into them. Tell why you changed something unless it is completely self-evident; this is particularly important for reverts. Use the Qt commit template. Follow the summary + description message style and use footers to reference JIRA issues, collaborators, etc. and consider the generic Git commit message guidelines.
    3. Commit often. Use git gui and git rebase -i extensively to get your unpublished history into shape. Note that pushing to your private clone does not count as publishing and is a perfectly valid way to solicit an early review or to make a backup of a work in progress. Further reading: Understanding the Git Workflow
    4. Avoid unnecessary merges. Use git pull —rebase unless you have an unpushed "proper" merge. Our Gerrit permits only trusted Merge Masters to push merge commits.
  9. Review your changes before you push to Gerrit. git log [--stat] [--summary] [-p] @{u}.. and gitk are your friends.
  10. Make sure to follow the Branch Guidelines. Submit against the lowest applicable branch from which a release is still planned. Cherry-picks ("backports") are strongly discouraged (use the -x option if you still do), while forward-merging to more recent branches happens "automatically" on a regular basis.
  11. And most importantly: use your brain.

Next, follow the Review Policy.

Change Log

If the change is significant and affects many users, compatibility, or is a noteworthy feature, you must add a ChangeLog tag. Conversely, do not add ChangeLog entries that are not relevant to users.

  • A [ChangeLog] entry can be multiple lines long and ends with an empty line.
  • Try to integrate the changelog entry into the surrounding commit message to avoid redundancy.
  • Iff the Git repository contains multiple modules, use the module name to indicate the area of the change e.g. [QtCore].
  • Optionally specify a class or subtopic [QtNetwork][QSslSocket]
  • Other common tags are:
    • [General]
    • [Important Behavior Changes]
    • [Platform Specific Changes]
    • [Windows]
    • [OS X]
    • [Linux/XCB]
  • In summary, the entry should look like this:
    [ChangeLog][module][class/topic] description of the really important
    change that was just made (on several lines).
    • The description should use either simple past ("Fixed …") or be explicit about referring to the current state ("… does now …").
    • Make sure the entry is reasonably self-contained. If you fail to formulate a meaningful description, it's probably not useful information to start with.

Additional notes

  • When you commit translations, use "make commit-ts" instead of "git commit" to keep the line number information out of the committed files.
  • For non-native English speakers who'd like any written English (code comments and documentation in general) to be reviewed, feel free to add one of the people listed under the Approvers and Editors section here.