Code Reviews: Difference between revisions

From Qt Wiki
Jump to navigation Jump to search
No edit summary
 
(redirect to qt contribution guidelines, as it's basically redundant now)
 
(4 intermediate revisions by 2 users not shown)
Line 1: Line 1:
'''Note: this page is partly redundant with [[Qt-Contribution-Guidelines|Qt Contribution Guidelines]], which gives a better introduction.'''
#REDIRECT [[Qt Contribution Guidelines]]
 
=Code Reviews=
 
'''Participating in the Qt project by contributing and reviewing'''
 
This Wiki provides information on how to contribute and review code. It also provides directions to the information on Gerrit and Git used for developing Qt.
 
==Overview of the technical landscape==
 
The servers, database, and tools related to the Qt Open Governance code contribution process as well as their interactions are illustrated in the adjacent picture. The process has been designed to provide increased transparency and quick pass-through for code contributions, and its core is the easy-to-use code review tool, Gerrit. The tool is available for everybody, and besides using it for its main purpose of contributors and feedback gathering, it can be utilized for studying the code of the other contributors.
 
Here’s an overview diagram of the process:
 
[[Image:New_Landscape.png]]
 
In order to gain access and log in to Gerrit, a contributor needs to have an existing <span class="caps">JIRA</span> account. A new account can be obtained from https://bugreports.qt.io/.
 
Gerrit tool is used in the Qt Project for approval of changes and both automated quality checks and human reviews. It is hosted under the codereview.qt.io domain. Contributions are sent as patches to Gerrit with the Git push command, with target ref as refs/for/&lt;branch name&gt;. There will be no direct pushing into a branch. Repository can be accessed with <span class="caps">SSH</span> on port 29418.
 
Continuous Integration (CI) system is used for building and testing. CI fetches reviewed changes from Gerrit, runs build &amp; test for the changes and transfers the changes passed further to the public tip. Staging branches are stored in the Git repository of Gerrit.
 
In addition to Gerrit’s Git repository, the repository is pushed to Gitorious repository on qt.gitorious.org. That repository is read-only.
 
==Using Git==
 
* [[Git Installation]]
* [[Get The Source|Get the source]]
* [[Git Introduction|Developing and maintaining patches on top of Qt with Git]]
 
==Using Gerrit==
 
* [[Setting-up-Gerrit|Setting up your Gerrit environment]]
* [[Gerrit-Introduction|Introduction to Gerrit]]
** Contribution creation and uploading
** Requesting and receiving contribution feedback
** Approving and abandoning
** Continuous integration and staging
** Merging feature branches
** Access rights
** Providing feedback about the set up
* [http://qt.io/videos/watch/gerrit_contributing_and_reviewing_a_change <span class="caps">VIDEO</span>: Contributing and reviewing a change, 3min 15sec] ''[qt.io]''
 
==Troubleshooting==
 
===Checklist for contributing===
 
In order to contribute person needs to have:
 
* <span class="caps">JIRA</span> account: can be created from https://bugreports.qt.io
* Register to Gerrit using e-mail https://codereview.qt.io
* Accept the Contributor License Agreement
** Individual contributor does that in Gerrit (Settings -&gt; Agreements -&gt; New Contributor Agreement)
** Corporate contributor needs to fill in <span class="caps">CLA</span> for corporate contributor
* If code is contributed user needs to clone the source code (see: [[Get The Source|Get the source]])
 
===Troubleshooting Table===
 
{| class="infotable line"
! Symptom
! Cause
! Solution
|}
 
{| class="infotable line"
| When I click the email verification link in Outlook, Gerrit told “Invalid token”
| Outlook doesn’t activate the link correctly. Specifically, the last two “h2. “ are omitted when clicking on the link
| Copy and paste the link into a web browser, instead of clicking on it
|-
| “git push” fails with: “error: failed to push some refs to …”
| Many possible causes. Generic error message.
| Please read the rest of the output of “git push” to determine the root cause (especially lines beginning with “remote:”).
|-
| “git push” fails with: “<span class="caps">ERROR</span>: missing Change-Id in commit message”.
| You tried to push a commit whose commit message didn’t contain a “Change-Id” line. This may be:
| .
|-
| .
| commit is new
|
To prevent the problem occurring for new commits, install the commit message hook according to the instructions under [[Setting-up-Gerrit|Setting up Gerrit Local Setup]]. Then use git commit —amend to fix up the current commit (just exit the commit message editor without changing anything).
|-
| .
| a commit you have cherry-picked from outside of Gerrit
| Use one of these tools to add a Change-Id (roughly listed in order of expertise required): git commit —amend, git-edit-commit (from devtools/mainline.git), git rebase —interactive
|-
| “git push” fails with “<span class="caps">ERROR</span>: you are not allowed to upload merges”
| You tried to push a merge commit for review, which is not allowed by default.
| Contact one of the administrators listed in the Gerrit Registration section and ask for permission to do merges. The primary reason for this is to reduce the risk of accidental pushing of merges (e.g. by users who use Git in a <span class="caps">CVS</span>-like manner, without learning about effective usage of merges and rebases).
|-
| I can’t give +2 to anything, or submit or stage anything
| You’re not in the “Approvers” group.
|
Apply for approver status according to the [[The Qt Governance Model|Qt Governance Model]]. If your status was confirmed but not executed yet, contact one of the administrators listed in the Gerrit Registration section to add you to the “Approvers” group.
|-
| “Qt Sanity Bot” gave a -1 to my change!
| An automated sanity check bot found out that change contains some problem.
| First, really try to understand the problem. Don’t proceed until you understand the output of the bot. Then make an informed decision to either fix it (e.g. by amending your change with the fix and pushing it again), or ignore it. The sanity bot’s output is advisory in cases where it is determined heuristically.
|-
| “Qt Sanity Bot” gave a -2 to my change!
| An automated sanity check bot found out that change contains some serious problem.
| First, really try to understand the problem. If you are convinced that the bot made a mistake, contact a member of the Administrators group on Gerrit.
|-
| “Qt Continuous Integration System” said my change failed, but I’m sure my change is OK!
| The test procedure for your change failed. It could be:
| .
|-
| .
| a regression caused by someone else’s change which was tested at the same time
| Simply stage your change again.
|-
| .
| a regression caused by a change in some dependency of the module you’re working on (e.g. a qtbase bug breaking qtdeclarative)
| Fix the bug in the dependency, or ensure someone else fixes it, before re-staging your change.
|-
| .
| an unstable failure due to bugs in the code (e.g. an autotest which fails ~50% of the time, seemingly at random)
| Fix the instability in a separate commit before re-staging your change, or consult QA for advice.
|-
| .
| bugs or infrastructure problems in the CI system.
| Consult QA for advice.
|-
| .
| a regression genuinely caused by your change
| Reconsider if you should really be “sure” that your change is OK. For a project the size of Qt, it is common for changes in one part of the code to affect other parts of the code in unexpected ways.
|}
 
===A Real World Troubleshooting Example===
 
This is a lightly edited log of troubleshooting an attempt to push a commit to gerrit for Qt 4.8.
 
The contents of the pastebin were as follows:
 
===Categories:===
 
* [[:Category:Developing Qt|Developing_Qt]]
** [[:Category:Developing Qt::Guidelines|Guidelines]]

Latest revision as of 19:37, 3 March 2015

Redirect to:

Retrieved from "https://wiki.qt.io/index.php?title=Code_Reviews&oldid=10925"