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)
 
(2 intermediate revisions by the same user not shown)
Line 1: Line 1:
[[Category:Developing_Qt::Guidelines]]
#REDIRECT [[Qt Contribution Guidelines]]
 
'''Note: this page is partly redundant with [[Qt-Contribution-Guidelines]], which gives a better introduction.'''
 
= 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:http://qt.io/uploads/image_upload/New_Landscape.png |http://qt.io/uploads/image_upload/New_Landscape.png ]]
 
In order to gain access and log in to Gerrit, a contributor needs to have an existing JIRA 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/<branch name>. There will be no direct pushing into a branch. Repository can be accessed with SSH 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|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
* "VIDEO: Contributing and reviewing a change, 3min 15sec":http://qt.io/videos/watch/gerrit_contributing_and_reviewing_a_change
 
== Troubleshooting ==
 
=== Checklist for contributing ===
 
In order to contribute person needs to have:
 
* JIRA 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 -> Agreements-> New Contributor Agreement)
** Corporate contributor needs to fill in CLA for corporate contributor
* If code is contributed user needs to clone the source code (see: [[Get_The_Source|Get the source]])
 
=== Troubleshooting Table ===
 
|''. Symptom |''. Cause |_. Solution|
 
{|
|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: "ERROR: 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
|-
|.
|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 "ERROR: 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 CVS-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 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.
 
<code>
[20:16] <curius> git gives me this "error: failed to push some
refs to 'ssh://giotis.nikos@codereview.qt.io:29418/qt/qt'"
[20:16] <richmoore1> curius: login to gerrit
[20:16] <richmoore1> curius: click settings at the top right
[20:17] <richmoore1> curius: click Agreements on the left at the bottom of the list
[20:17] <curius> verified
[20:17] <richmoore1> ok, good
[20:17] <richmoore1> now click ssh public keys
[20:18] <richmoore1> do you see your key?
[20:19] <curius> yes
[20:19] <richmoore1> okay
[20:20] <richmoore1> so we know that in theory you can push but there's likely some problem with /what/
your trying to push, or where it's going to
[20:20] <peppe> curius: Change-Id installed?
[20:21] <curius> Change-Id? don't know what that is :)
[20:21] <curius> the git/hook thing?
[20:21] <richmoore1> yes, it should be in your commit message
[20:21] <richmoore1> do you see a line like Change-Id: Iffbdf8f5ec366d9f41debcf8301340b5ff2207bb
[20:21] <qt_gerrit> https://codereview.qt.io/#q,Iffbdf8f5ec366d9f41debcf8301340b5ff2207bb,n,z
[20:22] <curius> oh.. ok, there is a change-id, yes
[20:23] <curius> Change-Id: Ibc7546c522506b2c4e7a729a8de319aea788b662
[20:23] <qt_gerrit> https://codereview.qt.io/#q,Ibc7546c522506b2c4e7a729a8de319aea788b662,n,z
[20:24] <richmoore1> okay, so we know you have the git hooks working
[20:25] <richmoore1> curius: the problem is probably that you've fallen behind the current tree
[20:25] <richmoore1> curius: do a git pull to get yourself up to date
[20:26] <richmoore1> curius: you may need to tweak your change to make it work after this
[20:28] <curius> git pull ssh://giotis.nikos@codereview.qt.io:29418/qt/qt
[20:28] <curius> this says 'Already up-to-date'
[20:29] <richmoore1> okay, what else did it say when you tried to push?
[20:30] <curius> http://pastebin.com/NqbJZBaS
</code>
 
The contents of the pastebin were as follows:
 
<code>
Counting objects: 7, done.
Delta compression using up to 4 threads.
Compressing objects: 100% (4/4), done.
Writing objects: 100% (4/4), 555 bytes, done.
Total 4 (delta 3), reused 0 (delta 0)
remote: Resolving deltas: 0% (0/3)
To ssh://giotis.nikos@codereview.qt.io:29418/qt/qt
! [remote rejected] HEAD -> refs/for/master (branch master not found)
error: failed to push some refs to 'ssh://giotis.nikos@codereview.qt.io
:29418/qt/qt'
</code>
 
<code>
[20:31] <richmoore1> aha, see line 8?
[20:31] <curius> yes? branch master not found ?
[20:32] <richmoore1> yes, that's the problem. iirc you're trying to push to qt 4.8 yes?
[20:32] <curius> yes, 4.8 master
[20:32] <richmoore1> okay, so we need to find out what the branch is
[20:33] <richmoore1> answer 4.8
[20:33] <curius> on branch 4.8
[20:33] <curius> yes
[20:33] <curius> ohh.. refs/for/4.8 ??
[20:33] <richmoore1> yep
[20:34] <curius> thank you :D
[20:34] <curius> http://codereview.qt.io/#change,15262
[20:34] <richmoore1> bingo :-)
[20:35] <curius> who's going to review it? it's very very trivial fix?
[20:35] <richmoore1> it's qmake. so add ossi
[20:36] <richmoore1> or i can add him for you if you want
[20:36] <curius> yes, please :)
[20:36] <curius> add him
[20:36] <richmoore1> done
[20:37] <curius> ahhh, nice :) … and now i wait for ossi to see it, right?
[20:37] <richmoore1> yep
[20:37] <richmoore1> you'll get mailed when there's comments etc.
[20:38] <richmoore1> and you can push again using the same commit id (eg. via updating
your commit with git commit -amend to address any issues found during the review)

Latest revision as of 19:37, 3 March 2015

Redirect to:

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