| Joel Hutton | 9e60563 | 2019-02-25 15:18:56 +0000 | [diff] [blame^] | 1 | How do I update a Pull Request? |
| 2 | ------------------------------- |
| 3 | |
| 4 | Often it is necessary to update a Pull Request (PR) before it is merged. When |
| 5 | you push to the source topic branch of an open PR, the PR is automatically |
| 6 | updated with the new commits. |
| 7 | |
| 8 | If you need to modify existing commits in the PR (for example following review |
| 9 | comments), then use the ``--force`` option when pushing. Any comments that apply |
| 10 | to previous versions of the PR are retained in the PR. Sometimes it may be |
| 11 | confusing whether comments apply to the current or a previous version of the PR, |
| 12 | especially if there are several rounds of rework. In this case, you may be asked |
| 13 | to close the PR and create a new one with the latest commits. The new PR should |
| 14 | have a version appended to the name (e.g. "My topic v2") and you should create a |
| 15 | link to the old PR so that reviewers can easily find previous versions. |
| 16 | |
| 17 | When the PR is finally merged, you will be given the option of deleting your |
| 18 | topic branch. It is recommended you delete this (and any previous topic branch |
| 19 | versions) to avoid polluting your fork with obsolete branches. |
| 20 | |
| 21 | How long will my Pull Request take to merge? |
| 22 | -------------------------------------------- |
| 23 | |
| 24 | This can vary a lot, depending on: |
| 25 | |
| 26 | * How important the Pull Request (PR) is considered by the TF maintainers. Where |
| 27 | possible, you should indicate the required timescales for merging the PR and |
| 28 | the impact of any delay. |
| 29 | |
| 30 | * The quality of the PR. PRs are likely to be merged quicker if they follow the |
| 31 | coding guidelines, have already had some code review, and have been |
| 32 | appropriately tested. Note that PRs from Arm engineers go through an internal |
| 33 | review process before appearing on GitHub, therefore may appear to be merged |
| 34 | more quickly. |
| 35 | |
| 36 | * The impact of the PR. For example, a PR that changes a key generic API is |
| 37 | likely to receive much greater scrutiny than a local change to a specific |
| 38 | platform port. |
| 39 | |
| 40 | * How much opportunity for external review is required. For example, the TF |
| 41 | maintainers may not wait for external review comments to merge trivial |
| 42 | bug-fixes but may wait up to a week to merge major changes, or ones requiring |
| 43 | feedback from specific parties. |
| 44 | |
| 45 | * How many other topics need to be integrated and the risk of conflict between |
| 46 | the topics. |
| 47 | |
| 48 | * Is there a code freeze in place in preparation for the release. Please refer |
| 49 | the `release information`_ for more details. |
| 50 | |
| 51 | * The workload of the TF maintainers. |
| 52 | |
| 53 | Feel free to add a comment to your PR to get an estimate of when it will |
| 54 | be merged. |
| 55 | |
| 56 | How long will it take for my merged Pull Request to go from ``integration`` to ``master``? |
| 57 | ------------------------------------------------------------------------------------------ |
| 58 | |
| 59 | This depends on how many concurrent Pull Requests (PRs) are being processed at |
| 60 | the same time. In simple cases where all potential regressions have already been |
| 61 | tested, the delay will be less than 1 day. If the TF maintainers are trying to |
| 62 | merge several things over the course of a few days, it might take up to a week. |
| 63 | Typically, it will be 1-2 days. |
| 64 | |
| 65 | The worst case is if the TF maintainers are trying to make a release while also |
| 66 | receiving PRs that will not be merged into the release. In this case, the PRs |
| 67 | will be merged onto ``integration``, which will temporarily diverge from the |
| 68 | release branch. The ``integration`` branch will be rebased onto ``master`` after |
| 69 | the release, and then ``master`` will be fast-forwarded to ``integration`` 1-2 |
| 70 | days later. This whole process could take up 4 weeks. Please refer the `release |
| 71 | information`_ for code freeze dates. The TF maintainers will inform the PR owner |
| 72 | if this is going to happen. |
| 73 | |
| 74 | It is OK to create a PR based on commits that are only available in |
| 75 | ``integration`` or another PR, rather than ``master``. There is a risk that the |
| 76 | dependency commits will change (for example due to PR rework or integration |
| 77 | problems). If this happens, the dependent PR will need reworking. |
| 78 | |
| 79 | What are these strange comments in my Pull Request? |
| 80 | --------------------------------------------------- |
| 81 | |
| 82 | For example, comments like "Can one of the admins verify this patch?" or "test |
| 83 | this please". These are associated with Arm's Continuous Integration |
| 84 | infrastructure and can be safely ignored. Those who are curious can see the |
| 85 | documentation for `this Jenkins plugin`_ for more details. |
| 86 | |
| 87 | .. _release information: release-information |
| 88 | .. _this Jenkins plugin: https://wiki.jenkins-ci.org/display/JENKINS/GitHub+pull+request+builder+plugin |