John DeNisco | 06dcd45 | 2018-07-26 12:45:10 -0400 | [diff] [blame] | 1 | .. _gitreview: |
| 2 | |
andrew | bcddab2 | 2018-08-06 17:40:31 -0400 | [diff] [blame] | 3 | ******************************* |
John DeNisco | 758dc46 | 2018-08-13 17:00:06 -0400 | [diff] [blame] | 4 | Getting a Patch Reviewed |
andrew | bcddab2 | 2018-08-06 17:40:31 -0400 | [diff] [blame] | 5 | ******************************* |
John DeNisco | 06dcd45 | 2018-07-26 12:45:10 -0400 | [diff] [blame] | 6 | |
John DeNisco | 758dc46 | 2018-08-13 17:00:06 -0400 | [diff] [blame] | 7 | This section describes how to get FD.io VPP sources reviewed and merged. |
John DeNisco | 06dcd45 | 2018-07-26 12:45:10 -0400 | [diff] [blame] | 8 | |
andrew | bcddab2 | 2018-08-06 17:40:31 -0400 | [diff] [blame] | 9 | Setup |
| 10 | ======== |
John DeNisco | 06dcd45 | 2018-07-26 12:45:10 -0400 | [diff] [blame] | 11 | |
andrew | bcddab2 | 2018-08-06 17:40:31 -0400 | [diff] [blame] | 12 | If you don't have a Linux Foundation ID, `create one here. <https://identity.linuxfoundation.org/>`_ |
John DeNisco | 06dcd45 | 2018-07-26 12:45:10 -0400 | [diff] [blame] | 13 | |
andrew | bcddab2 | 2018-08-06 17:40:31 -0400 | [diff] [blame] | 14 | With your Linux Foundation ID credentials sign into `Gerrit Code Review at gerrit.fd.io <https://gerrit.fd.io/r/login/%23%2Fq%2Fstatus%3Aopen>`_ |
John DeNisco | 06dcd45 | 2018-07-26 12:45:10 -0400 | [diff] [blame] | 15 | |
andrew | bcddab2 | 2018-08-06 17:40:31 -0400 | [diff] [blame] | 16 | `Install git-review, <https://www.mediawiki.org/wiki/Gerrit/git-review>`_ which is a "command-line tool for Git / Gerrit to submit a change or to fetch an existing one." |
John DeNisco | 06dcd45 | 2018-07-26 12:45:10 -0400 | [diff] [blame] | 17 | |
andrew | bcddab2 | 2018-08-06 17:40:31 -0400 | [diff] [blame] | 18 | If you're on Ubuntu, install keychain: |
| 19 | |
| 20 | .. code-block:: console |
| 21 | |
| 22 | $ sudo apt-get install keychain |
| 23 | |
| 24 | ssh keys |
| 25 | ------------- |
| 26 | |
| 27 | To get FD.io VPP documents reviewed the VPP repository should be cloned with ssh. You should be logged into Gerrit Code Review as noted above. |
| 28 | |
| 29 | Create your public and private ssh key with: |
John DeNisco | 06dcd45 | 2018-07-26 12:45:10 -0400 | [diff] [blame] | 30 | |
| 31 | .. code-block:: console |
| 32 | |
| 33 | $ ssh-keygen -t rsa |
| 34 | $ keychain |
| 35 | $ cat ~/.ssh/id_rsa.pub |
| 36 | |
andrew | bcddab2 | 2018-08-06 17:40:31 -0400 | [diff] [blame] | 37 | Copy **all** the contents of the public key (id_rsa.pub) output by the above **cat** command. Then go to your `SSH Public keys settings page <https://gerrit.fd.io/r/#/settings/ssh-keys>`_, click **Add Key ...**, paste your public key, and finally click **Add**. |
| 38 | |
| 39 | .. _clone-ssh: |
| 40 | |
| 41 | Clone with ssh |
| 42 | ============== |
| 43 | |
| 44 | Clone the repo with: |
John DeNisco | 06dcd45 | 2018-07-26 12:45:10 -0400 | [diff] [blame] | 45 | |
| 46 | .. code-block:: console |
| 47 | |
| 48 | $ git clone ssh://gerrit.fd.io:29418/vpp |
| 49 | $ cd vpp |
| 50 | |
andrew | bcddab2 | 2018-08-06 17:40:31 -0400 | [diff] [blame] | 51 | This will only work if the name of the user on your system matches your Gerrit username. |
John DeNisco | 06dcd45 | 2018-07-26 12:45:10 -0400 | [diff] [blame] | 52 | |
andrew | bcddab2 | 2018-08-06 17:40:31 -0400 | [diff] [blame] | 53 | Otherwise, clone with: |
| 54 | |
| 55 | .. code-block:: console |
| 56 | |
John DeNisco | 2ba9dcf | 2018-08-23 14:04:22 -0400 | [diff] [blame] | 57 | $ git clone ssh://<YOUR_GERRIT_USERNAME>@gerrit.fd.io:29418/vpp |
andrew | bcddab2 | 2018-08-06 17:40:31 -0400 | [diff] [blame] | 58 | $ cd vpp |
| 59 | |
John DeNisco | ce96dda | 2018-08-14 16:04:09 -0400 | [diff] [blame] | 60 | When attempting to clone the repo Git will prompt you asking if you want to add the Server Host Key to the list of known hosts. Enter **yes** and press the **Enter** key. |
andrew | bcddab2 | 2018-08-06 17:40:31 -0400 | [diff] [blame] | 61 | |
| 62 | Git Review |
| 63 | =========== |
| 64 | |
John DeNisco | ce96dda | 2018-08-14 16:04:09 -0400 | [diff] [blame] | 65 | The VPP documents use the gerrit server, and git review for submitting and fetching patches. |
andrew | bcddab2 | 2018-08-06 17:40:31 -0400 | [diff] [blame] | 66 | |
| 67 | |
| 68 | New patch |
| 69 | ----------------- |
| 70 | |
John DeNisco | ce96dda | 2018-08-14 16:04:09 -0400 | [diff] [blame] | 71 | When working with a new patch, use the following commands to get your patch reviewed. |
andrew | bcddab2 | 2018-08-06 17:40:31 -0400 | [diff] [blame] | 72 | |
John DeNisco | ce96dda | 2018-08-14 16:04:09 -0400 | [diff] [blame] | 73 | Make sure you have modified the correct files by issuing the following commands: |
John DeNisco | 06dcd45 | 2018-07-26 12:45:10 -0400 | [diff] [blame] | 74 | |
| 75 | .. code-block:: console |
| 76 | |
John DeNisco | 2d1a043 | 2018-07-26 16:21:31 -0400 | [diff] [blame] | 77 | $ git status |
andrew | bcddab2 | 2018-08-06 17:40:31 -0400 | [diff] [blame] | 78 | $ git diff |
| 79 | |
John DeNisco | 758dc46 | 2018-08-13 17:00:06 -0400 | [diff] [blame] | 80 | Then add and commit the patch. You may want to add a tag to the commit comments. |
John DeNisco | 2ba9dcf | 2018-08-23 14:04:22 -0400 | [diff] [blame] | 81 | For example for a document with only patches you should add the tag **docs:**. |
andrew | bcddab2 | 2018-08-06 17:40:31 -0400 | [diff] [blame] | 82 | |
| 83 | .. code-block:: console |
| 84 | |
John DeNisco | 2d1a043 | 2018-07-26 16:21:31 -0400 | [diff] [blame] | 85 | $ git add <filename> |
John DeNisco | 742000a | 2020-07-06 12:27:58 -0400 | [diff] [blame^] | 86 | $ git commit -s |
| 87 | |
| 88 | The commit comment should have something like the following comment: |
| 89 | |
| 90 | .. code-block:: console |
| 91 | |
| 92 | docs: A brief description of the commit |
| 93 | |
| 94 | Type: Improvement (The type of commit this could be: Improvement, Fix or Feature) |
| 95 | |
| 96 | A detailed description of the commit could go here. |
| 97 | |
| 98 | Push the patch for review. |
| 99 | |
| 100 | .. code-block:: console |
| 101 | |
John DeNisco | 2d1a043 | 2018-07-26 16:21:31 -0400 | [diff] [blame] | 102 | $ git review |
John DeNisco | 06dcd45 | 2018-07-26 12:45:10 -0400 | [diff] [blame] | 103 | |
andrew | bcddab2 | 2018-08-06 17:40:31 -0400 | [diff] [blame] | 104 | If you are creating a draft, meaning you do not want your changes reviewed yet, do the following: |
John DeNisco | 06dcd45 | 2018-07-26 12:45:10 -0400 | [diff] [blame] | 105 | |
andrew | bcddab2 | 2018-08-06 17:40:31 -0400 | [diff] [blame] | 106 | .. code-block:: console |
John DeNisco | 06dcd45 | 2018-07-26 12:45:10 -0400 | [diff] [blame] | 107 | |
John DeNisco | 2d1a043 | 2018-07-26 16:21:31 -0400 | [diff] [blame] | 108 | $ git review -D |
John DeNisco | 06dcd45 | 2018-07-26 12:45:10 -0400 | [diff] [blame] | 109 | |
andrew | bcddab2 | 2018-08-06 17:40:31 -0400 | [diff] [blame] | 110 | After submitting a review, reset where the HEAD is pointing to with: |
John DeNisco | 06dcd45 | 2018-07-26 12:45:10 -0400 | [diff] [blame] | 111 | |
andrew | bcddab2 | 2018-08-06 17:40:31 -0400 | [diff] [blame] | 112 | .. code-block:: console |
| 113 | |
| 114 | $ git reset --hard origin/master |
| 115 | |
| 116 | Existing patch |
| 117 | ----------------------- |
| 118 | |
| 119 | The "change number" used below is in the URL of the review. |
| 120 | |
John DeNisco | 758dc46 | 2018-08-13 17:00:06 -0400 | [diff] [blame] | 121 | After clicking an individual review, the change number can be found in the URL at "https://gerrit.fd.io/r/#/c/<*CHANGE_NUMBER*>/" |
andrew | bcddab2 | 2018-08-06 17:40:31 -0400 | [diff] [blame] | 122 | |
| 123 | To view an existing patch: |
| 124 | |
| 125 | .. code-block:: console |
| 126 | |
| 127 | $ git review -d <change number> |
| 128 | $ git status |
| 129 | $ git diff |
| 130 | |
| 131 | .. caution:: |
| 132 | |
| 133 | If you have made changes and do "git review -d <change number>", your current |
| 134 | changes will try to be stashed so that the working tree can change to the review branch |
| 135 | you specified. If you want to make sure you don't lose your changes, clone another Gerrit |
| 136 | repo into a new directory using the cloning steps shown in :ref:`clone-ssh`, and perform |
| 137 | "git review -d <change number>" in this new directory. |
| 138 | |
| 139 | To modify an existing patch, make sure you modified the correct files, and apply the patch with: |
| 140 | |
| 141 | .. code-block:: console |
| 142 | |
| 143 | $ git review -d <change number> |
| 144 | $ git status |
| 145 | $ git diff |
| 146 | |
| 147 | $ git add <filename> |
| 148 | $ git commit --amend |
| 149 | $ git review |
| 150 | |
John DeNisco | 2ba9dcf | 2018-08-23 14:04:22 -0400 | [diff] [blame] | 151 | When you're done viewing or modifying a branch, get back to the master branch by entering: |
John DeNisco | 06dcd45 | 2018-07-26 12:45:10 -0400 | [diff] [blame] | 152 | |
| 153 | .. code-block:: console |
| 154 | |
John DeNisco | 2d1a043 | 2018-07-26 16:21:31 -0400 | [diff] [blame] | 155 | $ git reset --hard origin/master |
| 156 | $ git checkout master |
John DeNisco | 758dc46 | 2018-08-13 17:00:06 -0400 | [diff] [blame] | 157 | |
Dave Barach | 8a69289 | 2018-10-24 09:23:23 -0400 | [diff] [blame] | 158 | Patch Conflict Resolution |
| 159 | ------------------------- |
John DeNisco | 758dc46 | 2018-08-13 17:00:06 -0400 | [diff] [blame] | 160 | |
Dave Barach | 8a69289 | 2018-10-24 09:23:23 -0400 | [diff] [blame] | 161 | Two different patch conflict scenarios arise from time to |
| 162 | time. Sometime after uploading a patch to https://gerrit.fd.io, the |
| 163 | gerrit UI may show a patch status of "Merge Conflict." |
| 164 | |
| 165 | Or, you may attempt to upload a new patch-set via "git review," only to |
| 166 | discover that the gerrit server won't allow the upload due to an upstream |
| 167 | merge conflict. |
| 168 | |
| 169 | In both cases, it's [usually] fairly simple to fix the problem. You |
| 170 | need to rebase the patch onto master/latest. Details vary from case to |
| 171 | case. |
| 172 | |
| 173 | Here's how to rebase a patch previously uploaded to the Gerrit server |
| 174 | which now has a merge conflict. In a fresh workspace cloned from |
| 175 | master/latest, do the following: |
John DeNisco | 758dc46 | 2018-08-13 17:00:06 -0400 | [diff] [blame] | 176 | |
| 177 | .. code-block:: console |
| 178 | |
John DeNisco | 2ba9dcf | 2018-08-23 14:04:22 -0400 | [diff] [blame] | 179 | $ git-review -d <*Gerrit change #*> |
John DeNisco | 758dc46 | 2018-08-13 17:00:06 -0400 | [diff] [blame] | 180 | $ git rebase origin/master |
| 181 | while (conflicts) |
| 182 | <fix conflicts> |
| 183 | $ git rebase --continue |
| 184 | $ git review |
| 185 | |
Dave Barach | 8a69289 | 2018-10-24 09:23:23 -0400 | [diff] [blame] | 186 | In the upload-failure case, use caution: carefully **save your work** |
| 187 | before you do anything else! |
John DeNisco | 758dc46 | 2018-08-13 17:00:06 -0400 | [diff] [blame] | 188 | |
Dave Barach | 8a69289 | 2018-10-24 09:23:23 -0400 | [diff] [blame] | 189 | Rebase your patch and try again. Please **do not** re-download ["git |
| 190 | review -d"] the patch from the gerrit server...: |
| 191 | |
| 192 | .. code-block:: console |
| 193 | |
| 194 | $ git rebase origin/master |
| 195 | while (conflicts) |
| 196 | <fix conflicts> |
| 197 | $ git rebase --continue |
| 198 | $ git review |
John DeNisco | 758dc46 | 2018-08-13 17:00:06 -0400 | [diff] [blame] | 199 | |