pr_workflow.rst 25 KB

123456789101112131415161718192021222324252627282930313233343536373839404142434445464748495051525354555657585960616263646566676869707172737475767778798081828384858687888990919293949596979899100101102103104105106107108109110111112113114115116117118119120121122123124125126127128129130131132133134135136137138139140141142143144145146147148149150151152153154155156157158159160161162163164165166167168169170171172173174175176177178179180181182183184185186187188189190191192193194195196197198199200201202203204205206207208209210211212213214215216217218219220221222223224225226227228229230231232233234235236237238239240241242243244245246247248249250251252253254255256257258259260261262263264265266267268269270271272273274275276277278279280281282283284285286287288289290291292293294295296297298299300301302303304305306307308309310311312313314315316317318319320321322323324325326327328329330331332333334335336337338339340341342343344345346347348349350351352353354355356357358359360361362363364365366367368369370371372373374375376377378379380381382383384385386387388389390391392393394395396397398399400401402403404405406407408409410411412413414415416417418419420421422423424425426427428429430431432433434435436437438439440441442443444445446447448449450451452453454455456457458459460461462463464465466467468469470471472473474475476477478479480481482483484485486487488489490491492493494495496497498499500501502503504505506507508509510511512513514515516517518519520521522523524525526527528529530531532533534535536537538539540541542543544545546547548549550551552553554555556557558559560561562563564565566567568569570571572573574575576577578579580581582583
  1. .. _doc_pr_workflow:
  2. Pull request workflow
  3. =====================
  4. .. highlight:: shell
  5. The so-called "PR workflow" used by Godot is common to many projects using
  6. Git, and should be familiar to veteran free software contributors. The idea
  7. is that only a small number (if any) commit directly to the *master* branch.
  8. Instead, contributors *fork* the project (i.e. create a copy of it, which
  9. they can modify as they wish), and then use the GitHub interface to request
  10. a *pull* from one of their fork's branches to one branch of the original
  11. (often named *upstream*) repository.
  12. The resulting *pull request* (PR) can then be reviewed by other contributors,
  13. which might approve it, reject it, or most often request that modifications
  14. be done. Once approved, the PR can then be merged by one of the core
  15. developers, and its commit(s) will become part of the target branch (usually
  16. the *master* branch).
  17. We will go together through an example to show the typical workflow and
  18. associated Git commands. But first, let's have a quick look at the
  19. organization of Godot's Git repository.
  20. Git source repository
  21. ---------------------
  22. The `repository on GitHub <https://github.com/godotengine/godot>`_ is a
  23. `Git <https://git-scm.com>`_ code repository together with an embedded
  24. issue tracker and PR system.
  25. .. note:: If you are contributing to the documentation, its repository can
  26. be found `here <https://github.com/godotengine/godot-docs>`_.
  27. The Git version control system is the tool used to keep track of successive
  28. edits to the source code - to contribute efficiently to Godot, learning the
  29. basics of the Git command line is *highly* recommended. There exist some
  30. graphical interfaces for Git, but they usually encourage users to take bad
  31. habits regarding the Git and PR workflow, and we therefore recommend not to
  32. use them. In particular, we advise not to use GitHub's online editor for code
  33. contributions (although it's tolerated for small fixes or documentation changes)
  34. as it enforces one commit per file and per modification,
  35. which quickly leads to PRs with an unreadable Git history (especially after peer review).
  36. .. seealso:: The first sections of Git's "Book" are a good introduction to
  37. the tool's philosophy and the various commands you need to
  38. master in your daily workflow. You can read them online on the
  39. `Git SCM <https://git-scm.com/book/en/v2>`_ website.
  40. You can also try out `GitHub's interactive guide <https://try.github.io/>`__.
  41. The branches on the Git repository are organized as follows:
  42. - The ``master`` branch is where the development of the next major version
  43. occurs. As a development branch, it can be unstable
  44. and is not meant for use in production. This is where PRs should be done
  45. in priority.
  46. - The stable branches are named after their version, e.g. ``3.1`` and ``2.1``.
  47. They are used to backport bugfixes and enhancements from the ``master``
  48. branch to the currently maintained stable release (e.g. 3.1.2 or 2.1.6).
  49. As a rule of thumb, the last stable branch is maintained until the next
  50. minor version (e.g. the ``3.0`` branch was maintained until the release of
  51. Godot 3.1).
  52. If you want to make PRs against a maintained stable branch, please check
  53. first if your changes are also relevant for the ``master`` branch, and if so
  54. make the PR for the ``master`` branch in priority. Release managers can then
  55. cherry-pick the fix to a stable branch if relevant.
  56. - There might occasionally be feature branches, usually meant to be merged into
  57. the ``master`` branch at some time.
  58. Forking and cloning
  59. -------------------
  60. The first step is to *fork* the `godotengine/godot <https://github.com/godotengine/godot>`_
  61. repository on GitHub. To do so, you will need to have a GitHub account and to
  62. be logged in. In the top right corner of the repository's GitHub page, you
  63. should see the "Fork" button as shown below:
  64. .. image:: img/github_fork_button.png
  65. Click it, and after a while you should be redirected to your own fork of the
  66. Godot repo, with your GitHub username as namespace:
  67. .. image:: img/github_fork_url.png
  68. You can then *clone* your fork, i.e. create a local copy of the online
  69. repository (in Git speak, the *origin remote*). If you haven't already,
  70. download Git from `its website <https://git-scm.com>`_ if you're using Windows or
  71. macOS, or install it through your package manager if you're using Linux.
  72. .. note:: If you are on Windows, open Git Bash to type commands. macOS and Linux users
  73. can use their respective terminals.
  74. To clone your fork from GitHub, use the following command:
  75. ::
  76. $ git clone https://github.com/USERNAME/godot
  77. .. note:: In our examples, the "$" character denotes the command line prompt
  78. on typical UNIX shells. It is not part of the command and should
  79. not be typed.
  80. After a little while, you should have a ``godot`` directory in your current
  81. working directory. Move into it using the ``cd`` command:
  82. ::
  83. $ cd godot
  84. We will start by setting up a reference to the original repository that we forked:
  85. ::
  86. $ git remote add upstream https://github.com/godotengine/godot
  87. $ git fetch upstream
  88. This will create a reference named ``upstream`` pointing to the original
  89. ``godotengine/godot`` repository. This will be useful when you want to pull new
  90. commits from its ``master`` branch to update your fork. You have another
  91. remote reference named ``origin``, which points to your fork (``USERNAME/godot``).
  92. You only need to do the above steps once, as long as you keep that local
  93. ``godot`` folder (which you can move around if you want, the relevant
  94. metadata is hidden in its ``.git`` subfolder).
  95. .. note:: *Branch it, pull it, code it, stage it, commit, push it, rebase
  96. it... technologic.*
  97. This bad take on Daft Punk's *Technologic* shows the general
  98. conception Git beginners have of its workflow: lots of strange
  99. commands to learn by copy and paste, hoping they will work as
  100. expected. And that's actually not a bad way to learn, as long as
  101. you're curious and don't hesitate to question your search engine
  102. when lost, so we will give you the basic commands to know when
  103. working in Git.
  104. In the following, we will assume as an example that you want to implement a feature in
  105. Godot's Project Manager, which is coded in the ``editor/project_manager.cpp``
  106. file.
  107. Branching
  108. ---------
  109. By default, the ``git clone`` should have put you on the ``master`` branch of
  110. your fork (``origin``). To start your own feature development, we will create
  111. a feature branch:
  112. ::
  113. # Create the branch based on the current branch (master)
  114. $ git branch better-project-manager
  115. # Change the current branch to the new one
  116. $ git checkout better-project-manager
  117. This command is equivalent:
  118. ::
  119. # Change the current branch to a new named one, based on the current branch
  120. $ git checkout -b better-project-manager
  121. If you want to go back to the ``master`` branch, you'd use:
  122. ::
  123. $ git checkout master
  124. You can see which branch you are currently on with the ``git branch``
  125. command:
  126. ::
  127. $ git branch
  128. 2.1
  129. * better-project-manager
  130. master
  131. Be sure to always go back to the ``master`` branch before creating a new branch,
  132. as your current branch will be used as the base for the new one. Alternatively,
  133. you can specify a custom base branch after the new branch's name:
  134. ::
  135. $ git checkout -b my-new-feature master
  136. Updating your branch
  137. --------------------
  138. This would not be needed the first time (just after you forked the upstream
  139. repository). However, the next time you want to work on something, you will
  140. notice that your fork's ``master`` is several commits behind the upstream
  141. ``master`` branch: pull requests from other contributors would have been merged
  142. in the meantime.
  143. To ensure there won't be conflicts between the feature you develop and the
  144. current upstream ``master`` branch, you will have to update your branch by
  145. *pulling* the upstream branch.
  146. ::
  147. $ git pull --rebase upstream master
  148. The ``--rebase`` argument will ensure that any local changes that you committed
  149. will be re-applied *on top* of the pulled branch, which is usually what we want
  150. in our PR workflow. This way, when you open a pull request, your own commits will
  151. be the only difference with the upstream ``master`` branch.
  152. While rebasing, conflicts may arise if your commits modified code that has been
  153. changed in the upstream branch in the meantime. If that happens, Git will stop at
  154. the conflicting commit and will ask you to resolve the conflicts. You can do so
  155. with any text editor, then stage the changes (more on that later), and proceed with
  156. ``git rebase --continue``. Repeat the operation if later commits have conflicts too,
  157. until the rebase operation completes.
  158. If you're unsure about what is going on during a rebase and you panic (no worry,
  159. we all do the first few times), you can abort the rebase with ``git rebase --abort``.
  160. You will then be back to the original state of your branch before calling
  161. ``git pull --rebase``.
  162. .. note:: If you omit the ``--rebase`` argument, you will instead create a merge
  163. commit which tells Git what to make of the two distinct branches. If any
  164. conflicts arise, they would be resolved all at once via this merge commit.
  165. While this is a valid workflow and the default behavior of ``git pull``,
  166. merge commits within PRs are frowned upon in our PR workflow. We only use
  167. them when merging PRs into the upstream branch.
  168. The philosophy is that a PR should represent the final stage of the changes
  169. made to the codebase, and we are not interested in mistakes and fixes that
  170. would have been done in intermediate stages before merging.
  171. Git gives us great tools to "rewrite the history" and make it as if we got
  172. things right the first time, and we're happy to use it to ensure that
  173. changes are easy to review and understand long after they have been merged.
  174. If you have already created a merge commit without using ``rebase``, or
  175. have made any other changes that have resulted in undesired history, the best option
  176. is to use an *interactive rebase* on the upstream branch. See the :ref:`dedicated
  177. section <doc_pr_workflow_rebase>` for instructions.
  178. .. tip:: If at any time you want to *reset* a local branch to a given commit or branch,
  179. you can do so with ``git reset --hard <commit ID>`` or
  180. ``git reset --hard <remote>/<branch>`` (e.g. ``git reset --hard upstream/master``).
  181. Be warned that this will remove any changes that you might have committed in
  182. this branch. If you ever lose commits by mistake, use the ``git reflog`` command
  183. to find the commit ID of the previous state that you would like to restore, and
  184. use it as argument of ``git reset --hard`` to go back to that state.
  185. Making changes
  186. --------------
  187. You would then do your changes to our example's
  188. ``editor/project_manager.cpp`` file with your usual development environment
  189. (text editor, IDE, etc.).
  190. By default, those changes are *unstaged*. The staging area is a layer between
  191. your working directory (where you make your modifications) and the local Git
  192. repository (the commits and all the metadata in the ``.git`` folder). To
  193. bring changes from the working directory to the Git repository, you need to
  194. *stage* them with the ``git add`` command, and then to commit them with the
  195. ``git commit`` command.
  196. There are various commands you should know to review your current work,
  197. before staging it, while it is staged, and after it has been committed.
  198. - ``git diff`` will show you the current unstaged changes, i.e. the
  199. differences between your working directory and the staging area.
  200. - ``git checkout -- <files>`` will undo the unstaged changes to the given
  201. files.
  202. - ``git add <files>`` will *stage* the changes on the listed files.
  203. - ``git diff --staged`` will show the current staged changes, i.e. the
  204. differences between the staging area and the last commit.
  205. - ``git reset HEAD <files>`` will *unstage* changes to the listed files.
  206. - ``git status`` will show you what are the currently staged and unstaged
  207. modifications.
  208. - ``git commit`` will commit the staged files. It will open a text editor
  209. (you can define the one you want to use with the ``GIT_EDITOR`` environment
  210. variable or the ``core.editor`` setting in your Git configuration) to let you
  211. write a commit log. You can use ``git commit -m "Cool commit log"`` to
  212. write the log directly.
  213. - ``git commit --amend`` lets you amend the last commit with your currently
  214. staged changes (added with ``git add``). This is the best option if you
  215. want to fix a mistake in the last commit (bug, typo, style issue, etc.).
  216. - ``git log`` will show you the last commits of your current branch. If you
  217. did local commits, they should be shown at the top.
  218. - ``git show`` will show you the changes of the last commit. You can also
  219. specify a commit hash to see the changes for that commit.
  220. That's a lot to memorize! Don't worry, just check this cheat sheet when you
  221. need to make changes, and learn by doing.
  222. Here's how the shell history could look like on our example:
  223. ::
  224. # It's nice to know where you're starting from
  225. $ git log
  226. # Do changes to the Project Manager with the nano text editor
  227. $ nano editor/project_manager.cpp
  228. # Find an unrelated bug in Control and fix it
  229. $ nano scene/gui/control.cpp
  230. # Review changes
  231. $ git status
  232. $ git diff
  233. # We'll do two commits for our unrelated changes,
  234. # starting by the Control changes necessary for the PM enhancements
  235. $ git add scene/gui/control.cpp
  236. $ git commit -m "Fix handling of margins in Control"
  237. # Check we did good
  238. $ git log
  239. $ git show
  240. $ git status
  241. # Make our second commit
  242. $ git add editor/project_manager.cpp
  243. $ git commit -m "Add a pretty banner to the Project Manager"
  244. $ git log
  245. With this, we should have two new commits in our ``better-project-manager``
  246. branch which were not in the ``master`` branch. They are still only local
  247. though, the remote fork does not know about them, nor does the upstream repo.
  248. Pushing changes to a remote
  249. ---------------------------
  250. That's where ``git push`` will come into play. In Git, a commit is always
  251. done in the local repository (unlike Subversion where a commit will modify
  252. the remote repository directly). You need to *push* the new commits to a
  253. remote branch to share them with the world. The syntax for this is:
  254. ::
  255. $ git push <remote> <local branch>[:<remote branch>]
  256. The part about the remote branch can be omitted if you want it to have the
  257. same name as the local branch, which is our case in this example, so we will
  258. do:
  259. ::
  260. $ git push origin better-project-manager
  261. Git will ask you for your username and password. For your password, enter your
  262. GitHub Personal Access Token (PAT). If you do not have a GitHub Personal Access
  263. Token, or do not have one with the correct permissions for your newly forked
  264. repository, you will need to create one. Follow this link to create your Personal
  265. Access Token: `Creating a personal access token
  266. <https://docs.github.com/en/authentication/keeping-your-account-and-data-secure/creating-a-personal-access-token>`_.
  267. After you have successfully verified your account using your PAT, the changes
  268. will be sent to your remote repository. If you check the fork's page on GitHub,
  269. you should see a new branch with your added commits.
  270. Issuing a pull request
  271. ----------------------
  272. When you load your fork's branch on GitHub, you should see a line saying
  273. *"This branch is 2 commits ahead of godotengine:master."* (and potentially some
  274. commits behind, if your ``master`` branch was out of sync with the upstream
  275. ``master`` branch).
  276. .. image:: img/github_fork_make_pr.png
  277. On that line, there is a "Pull request" link. Clicking it will open a form
  278. that will let you issue a pull request on the ``godotengine/godot`` upstream
  279. repository. It should show you your two commits, and state "Able to merge".
  280. If not (e.g. it has way more commits, or says there are merge conflicts),
  281. don't create the PR yet, something went wrong. Go to our
  282. `Godot Contributors Chat <https://chat.godotengine.org/>`_ and ask for support :)
  283. Use an explicit title for the PR and put the necessary details in the comment
  284. area. You can drag and drop screenshots, GIFs or zipped projects if relevant,
  285. to showcase what your work implements. Click "Create a pull request", and
  286. tadaa!
  287. Modifying a pull request
  288. ------------------------
  289. While it is reviewed by other contributors, you will often need to make
  290. changes to your yet-unmerged PR, either because contributors requested them,
  291. or because you found issues yourself while testing.
  292. The good news is that you can modify a pull request simply by acting on the
  293. branch you made the pull request from. You can e.g. make a new commit on that
  294. branch, push it to your fork, and the PR will be updated automatically:
  295. ::
  296. # Check out your branch again if you had changed in the meantime
  297. $ git checkout better-project-manager
  298. # Fix a mistake
  299. $ nano editor/project_manager.cpp
  300. $ git add editor/project_manager.cpp
  301. $ git commit -m "Fix a typo in the banner's title"
  302. $ git push origin better-project-manager
  303. However, be aware that in our PR workflow, we favor commits that bring the
  304. codebase from one functional state to another functional state, without having
  305. intermediate commits fixing up bugs in your own code or style issues. Most of
  306. the time, we will prefer a single commit in a given PR (unless there's a good
  307. reason to keep the changes separate). Instead of authoring a new commit,
  308. consider using ``git commit --amend`` to amend the previous commit with your
  309. fixes. The above example would then become:
  310. ::
  311. # Check out your branch again if you had changed in the meantime
  312. $ git checkout better-project-manager
  313. # Fix a mistake
  314. $ nano editor/project_manager.cpp
  315. $ git add editor/project_manager.cpp
  316. # --amend will change the previous commit, so you will have the opportunity
  317. # to edit its commit message if relevant.
  318. $ git commit --amend
  319. # As we modified the last commit, it no longer matches the one from your
  320. # remote branch, so we need to force push to overwrite that branch.
  321. $ git push --force origin better-project-manager
  322. .. Kept for compatibility with the previous title, linked in many PRs.
  323. .. _mastering-the-pr-workflow-the-rebase:
  324. .. _doc_pr_workflow_rebase:
  325. The interactive rebase
  326. ----------------------
  327. If you didn't follow the above steps closely to *amend* changes into a commit
  328. instead of creating fixup commits, or if you authored your changes without being
  329. aware of our workflow and Git usage tips, reviewers might request you to
  330. *rebase* your branch to *squash* some or all of the commits into one.
  331. Indeed, if some commits have been made following reviews to fix bugs, typos, etc.
  332. in the original commit, they are not relevant to a future changelog reader who
  333. would want to know what happened in the Godot codebase, or when and how a given
  334. file was last modified.
  335. To squash those extraneous commits into the main one, we will have to *rewrite
  336. history*. Right, we have that power. You may read that it's a bad practice, and
  337. it's true when it comes to branches of the upstream repo. But in your fork, you
  338. can do whatever you want, and everything is allowed to get neat PRs :)
  339. We will use the *interactive rebase* ``git rebase -i`` to do this. This
  340. command takes a commit ID or a branch name as argument, and will let you modify
  341. all commits between that commit/branch and the last one in your working branch,
  342. the so-called ``HEAD``.
  343. While you can give any commit ID to ``git rebase -i`` and review everything in
  344. between, the most common and convenient workflow involves rebasing on the
  345. upstream ``master`` branch, which you can do with:
  346. ::
  347. $ git rebase -i upstream/master
  348. .. note:: Referencing branches in Git is a bit tricky due to the distinction
  349. between remote and local branches. Here, ``upstream/master`` (with a
  350. `/`) is a local branch which has been pulled from the ``upstream``
  351. remote's ``master`` branch.
  352. Interactive rebases can only be done on local branches, so the `/`
  353. is important here. As the upstream remote changes frequently, your
  354. local ``upstream/master`` branch may become outdated, so you can
  355. update it with ``git fetch upstream master``. Contrarily to
  356. ``git pull --rebase upstream master`` which would update your
  357. currently checked out branch, ``fetch`` will only update the
  358. ``upstream/master`` reference (which is distinct from your local
  359. ``master`` branch... yes it's confusing, but you'll become familiar
  360. with this little by little).
  361. This will open a text editor (``vi`` by default, see
  362. `Git docs <https://git-scm.com/book/en/v2/Customizing-Git-Git-Configuration#_core_editor>`_
  363. to configure your favorite one) with something which may look like this:
  364. .. code-block:: text
  365. pick 1b4aad7 Add a pretty banner to the Project Manager
  366. pick e07077e Fix a typo in the banner's title
  367. The editor will also show instructions regarding how you can act on those
  368. commits. In particular, it should tell you that "pick" means to use that
  369. commit (do nothing), and that "squash" and "fixup" can be used to *meld* the
  370. commit in its parent commit. The difference between "squash" and "fixup" is
  371. that "fixup" will discard the commit log from the squashed commit. In our
  372. example, we are not interested in keeping the log of the "Fix a typo" commit,
  373. so we use:
  374. .. code-block:: text
  375. pick 1b4aad7 Add a pretty banner to the Project Manager
  376. fixup e07077e Fix a typo in the banner's title
  377. Upon saving and quitting the editor, the rebase will occur. The second commit
  378. will be melded into the first one, and ``git log`` and ``git show`` should
  379. now confirm that you have only one commit with the changes from both previous
  380. commits.
  381. But! You rewrote the history, and now your local and remote branches have
  382. diverged. Indeed, commit 1b4aad7 in the above example will have changed, and
  383. therefore got a new commit hash. If you try to push to your remote branch, it
  384. will raise an error:
  385. ::
  386. $ git push origin better-project-manager
  387. To https://github.com/akien-mga/godot
  388. ! [rejected] better-project-manager -> better-project-manager (non-fast-forward)
  389. error: failed to push some refs to 'https://akien-mga@github.com/akien-mga/godot'
  390. hint: Updates were rejected because the tip of your current branch is behind
  391. hint: its remote counterpart.
  392. This is reasonable behavior, Git will not let you push changes that would
  393. override remote content. But that's actually what we want to do here, so we
  394. will have to *force* it:
  395. ::
  396. $ git push --force origin better-project-manager
  397. And tadaa! Git will happily *replace* your remote branch with what you had
  398. locally (so make sure that's what you wanted, using ``git log``). This will
  399. also update the PR accordingly.
  400. Rebasing onto another branch
  401. ----------------------------
  402. If you have accidentally opened your PR on the wrong branch, or need to target another branch
  403. for some reason, you might need to filter out a lot of commits that differ between the old branch
  404. (for example ``4.2``) and the new branch (for example ``master``). This can make rebasing difficult
  405. and tedious. Fortunately ``git`` has a command just for this situation, ``git rebase --onto``.
  406. If your PR was created from the ``4.2`` branch and you want to update it to instead start at ``master``
  407. the following steps *should* fix this in one step:
  408. .. code-block:: text
  409. $ git rebase -i --onto master 4.2
  410. This will take all the commits on your branch *after* the ``4.2`` branch, and then splice them on top of ``master``,
  411. ignoring any commits from the ``4.2`` branch not on the ``master`` branch. You may still need to do some fixing, but
  412. this command should save you a lot of tedious work removing commits.
  413. Just like above for the interactive rebase you need to force push your branch to handle the different changes:
  414. ::
  415. $ git push --force origin better-project-manager
  416. Deleting a Git branch
  417. ---------------------
  418. After your pull request gets merged, there's one last thing you should do: delete your
  419. Git branch for the PR. There won't be issues if you don't delete your branch, but it's
  420. good practice to do so. You'll need to do this twice, once for the local branch and another
  421. for the remote branch on GitHub.
  422. To delete our better Project Manager branch locally, use this command:
  423. ::
  424. $ git branch -d better-project-manager
  425. Alternatively, if the branch hadn't been merged yet and we wanted to delete it anyway, instead
  426. of ``-d`` you would use ``-D``.
  427. Next, to delete the remote branch on GitHub use this command:
  428. ::
  429. $ git push origin -d better-project-manager
  430. You can also delete the remote branch from the GitHub PR itself, a button should appear once
  431. it has been merged or closed.