pr_workflow.rst 16 KB

123456789101112131415161718192021222324252627282930313233343536373839404142434445464748495051525354555657585960616263646566676869707172737475767778798081828384858687888990919293949596979899100101102103104105106107108109110111112113114115116117118119120121122123124125126127128129130131132133134135136137138139140141142143144145146147148149150151152153154155156157158159160161162163164165166167168169170171172173174175176177178179180181182183184185186187188189190191192193194195196197198199200201202203204205206207208209210211212213214215216217218219220221222223224225226227228229230231232233234235236237238239240241242243244245246247248249250251252253254255256257258259260261262263264265266267268269270271272273274275276277278279280281282283284285286287288289290291292293294295296297298299300301302303304305306307308309310311312313314315316317318319320321322323324325326327328329330331332333334335336337338339340341342343344345346347348349350351352353354355356357358359360361362363364365366367368369370371372373374375376377378379380381382383384385386387388389390391392393394395396397398399400401402403404405406407408409410
  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 branch 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. organisation 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. The Git version control system is the tool used to keep track of successive
  26. edits to the source code - to contibute efficiently to Godot, learning the
  27. basics of the Git command line is *highly* recommended. There exist some
  28. graphical interfaces for Git, but they usually encourage users to take bad
  29. habits regarding the Git and PR workflow, and we therefore recommend not to
  30. use them (especially GitHub's online editor).
  31. .. seealso:: The first sections of Git's "Book" are a good introduction to
  32. the tool's philosophy and the various commands you need to
  33. master in your daily workflow. You can read them online on the
  34. `Git SCM <https://git-scm.com/book/en/v2>`_ website.
  35. The branches on the Git repository are organized as follows:
  36. - The *master* branch is where the development of the next major version
  37. (3.0, 3.1, 4.0, etc.) occurs. As a development branch, it can be unstable
  38. and is not meant for use in production. This is where PRs should be done
  39. in priority.
  40. - The stable branches are named after their version, e.g. *2.0* and *2.1*.
  41. They are used to backport bugfixes and enhancements from the *master*
  42. branch to the currently maintained stable release (e.g. 2.0.1 or 2.1.3).
  43. As a rule of thumb, the last stable branch is maintained until the next
  44. major version (e.g. the *2.0* branch was maintained until the release of
  45. Godot 2.1).
  46. If you want to make PRs against a maintained stable branch, you will have
  47. to check if your changes are also relevant for the *master* branch.
  48. - There might be feature branches at time, usually meant to be merged into
  49. the *master* branch at some time.
  50. Forking and cloning
  51. -------------------
  52. The first step is to *fork* the `godotengine/godot <https://github.com/godotengine/godot>`_
  53. repository on GitHub. To do so, you will need to have a GitHub account and to
  54. be logged in. In the top right corner of the repository's GitHub page, you
  55. should see the "Fork" button as shown below:
  56. .. image:: /img/github_fork_button.png
  57. Click it, and after a while you should be redirected to your own fork of the
  58. Godot repo, with your GitHub username as namespace:
  59. .. image:: /img/github_fork_url.png
  60. You can then *clone* your fork, i.e. create a local copy of the online
  61. repository (in Git speak, the *origin remote*):
  62. ::
  63. $ git clone https://github.com/USERNAME/godot
  64. .. note:: In our examples, the "$" character denotes the command line prompt
  65. on typical UNIX shells. It is not part of the command and should
  66. not be typed.
  67. After a little while, you should have a ``godot`` directory in your current
  68. working directory. Move into it (``cd godot``), and we will set up a useful
  69. reference:
  70. ::
  71. $ git remote add upstream https://github.com/godotengine/godot
  72. $ git fetch upstream
  73. This will create a reference named *upstream* pointing to the original
  74. godotengine/godot repository. This will be useful when you want to pull new
  75. commits from its *master* branch to update your fork. You have another
  76. *remote* reference named *origin*, which points to your fork.
  77. You only need to do the above steps once, as long as you keep that local
  78. ``godot`` folder (which you can move around if you want, the relevant
  79. metadata is hidden in its ``.git`` subfolder).
  80. .. note:: *Branch it, pull it, code it, stage it, commit, push it, rebase
  81. it... technologic.*
  82. This bad take on Daft Punk's *Technologic* shows the general
  83. conception Git beginners have of its workflow: lots of strange
  84. commands to learn by copy and paste, hoping they will work as
  85. expected. And that's actually not a bad way to learn, as long as
  86. you're curious and don't hesitate to question your search engine
  87. when lost, so we will give you the basic commands to know when
  88. working in Git.
  89. In the following, we will assume that you want to implement a feature in
  90. Godot's project manager, which is coded in the ``editor/project_manager.cpp``
  91. file.
  92. Branching
  93. ---------
  94. By default, the ``git clone`` should have put you on the *master* branch of
  95. your fork (*origin*). To start your own feature development, we will create
  96. a feature branch:
  97. ::
  98. // Create the branch based on the current branch (master)
  99. $ git branch better-project-manager
  100. // Change the current branch to the new one
  101. $ git checkout better-project-manager
  102. This command is equivalent:
  103. ::
  104. // Change the current branch to a new named one, based on the current branch
  105. $ git checkout -b better-project-manager
  106. If you want to go back to the *master* branch, you'd use:
  107. ::
  108. $ git checkout master
  109. You can see which branch you are currently on with the ``git branch``
  110. command:
  111. ::
  112. $ git branch
  113. 2.1
  114. * better-project-manager
  115. master
  116. Updating your branch
  117. --------------------
  118. This would not be needed the first time, just after you forked the upstream
  119. repository. However, the next time you want to work on something, you will
  120. notice that your fork's *master* is several commits behind the upstream
  121. *master* branch: pull requests from other contributors would have been merged
  122. in the meantime.
  123. To ensure there won't be conflicts between the feature you develop and the
  124. current upstream *master* branch, you will have to update your branch by
  125. *pulling* the upstream branch.
  126. ::
  127. $ git pull upstream master
  128. However, if you had local commits, this method will create a so-called "merge
  129. commit", and you will soon hear from fellow contributors that those are not
  130. wanted in PRs. Then how to update the branch without creating a merge commit?
  131. You will have to use the ``--rebase`` option, so that your local commits are
  132. replayed on top of the updated upstream *master* branch. It will effectively
  133. modify the Git history of your branch, but that is for the greater good.
  134. Then command that you should (almost) always use is there:
  135. ::
  136. $ git pull --rebase upstream master
  137. Making changes
  138. --------------
  139. You would then do your changes to our example's
  140. ``editor/project_manager.cpp`` file with your usual development environment
  141. (text editor, IDE, etc.).
  142. By default, those changes are *unstaged*. The staging area is a layer between
  143. your working directory (where you make your modifications) and the local git
  144. repository (the commits and all the metadata in the ``.git`` folder). To
  145. bring changes from the working directory to the git repository, you need to
  146. *stage* them with the ``git add`` command, and then to commit them with the
  147. ``git commit`` command.
  148. There are various commands you should know to review your current work,
  149. before staging it, while it is staged, and after it has been committed.
  150. - ``git diff`` will show you the current unstaged changes, i.e. the
  151. differences between your working directory and the staging area.
  152. - ``git checkout -- <files>`` will undo the unstaged changes to the given
  153. files.
  154. - ``git add <files>`` will *stage* the changes on the listed files.
  155. - ``git diff --staged`` will show the current staged changes, i.e. the
  156. differences between the staging area and the last commit.
  157. - ``git reset HEAD <files>`` will *unstage* changes to the listed files.
  158. - ``git status`` will show you what are the currently staged and unstaged
  159. modifications.
  160. - ``git commit`` will commit the staged files. It will open a text editor
  161. (you can define the one you want to use with the ``GIT_EDITOR`` environment
  162. variable or the ``core.editor`` setting in your Git config) to let you
  163. write a commit log. You can use ``git commit -m "Cool commit log"`` to
  164. write the log directly.
  165. - ``git log`` will show you the last commits of your current branch. If you
  166. did local commits, they should be shown at the top.
  167. - ``git show`` will show you the changes of the last commit. You can also
  168. specify a commit hash to see the changes for that commit.
  169. That's a lot to memorise! Don't worry, just check this cheat sheet when you
  170. need to make changes, and learn by doing.
  171. Here's how the shell history could look like on our example:
  172. ::
  173. // It's nice to know where you're starting from
  174. $ git log
  175. // Do changes to the project manager
  176. $ nano editor/project_manager.cpp
  177. // Find an unrelated bug in Control and fix it
  178. $ nano scene/gui/control.cpp
  179. // Review changes
  180. $ git status
  181. $ git diff
  182. // We'll do two commits for our unrelated changes,
  183. // starting by the Control changes necessary for the PM enhancements
  184. $ git add scene/gui/control.cpp
  185. $ git commit -m "Fix handling of margins in Control"
  186. // Check we did good
  187. $ git log
  188. $ git show
  189. $ git status
  190. // Make our second commit
  191. $ git add editor/project_manager.cpp
  192. $ git commit -m "Add a pretty banner to the project manager"
  193. $ git log
  194. With this, we should have two new commits in our *better-project-manager*
  195. branch which were not in the *master* branch. They are still only local
  196. though, the remote fork does not know about them, nor does the upstream repo.
  197. Pushing changes to a remote
  198. ---------------------------
  199. That's where ``git push`` will come into play. In Git, a commit is always
  200. done in the local repository (unlike Subversion where a commit will modify
  201. the remote repository directly). You need to *push* the new commits to a
  202. remote branch to share them with the world. The syntax for this is:
  203. ::
  204. $ git push <remote> <local branch>[:<remote branch>]
  205. The part about the remote branch can be ommitted if you want it to have the
  206. same name as the local branch, which is our case in this example, so we will
  207. do:
  208. ::
  209. $ git push origin better-project-manager
  210. Git will ask you for your username and password, and the changes will be sent
  211. to your remote. If you check the fork's page on GitHub, you should see a new
  212. branch with your added commits.
  213. Issuing a pull request
  214. ----------------------
  215. When you load your fork's branch on GitHub, you should see a line saying
  216. "This branch is 2 commits ahead of godotengine:master." (and potentially some
  217. commits behind, if your *master* branch was out of sync with the upstream
  218. *master* branch.
  219. .. image:: /img/github_fork_make_pr.png
  220. On that line, there is a "Pull request" link. Clicking it will open a form
  221. that will let you issue a pull request on the godotengine/godot upstream
  222. repository. It should show you your two commits, and state "Able to merge".
  223. If not (e.g. it has way more commits, or says there are merge conflicts),
  224. don't create the PR, something went wrong. Go to IRC and ask for support :)
  225. Use an explicit title for the PR and put the necessary details in the comment
  226. area. You can drag and drop screenshots, gifs or zipped projects if relevant,
  227. to showcase what your work implements. Click "Create a pull request", and
  228. tadaa!
  229. Modifying a pull request
  230. ------------------------
  231. While it is reviewed by other contributors, you will often need to make
  232. changes to your yet-unmerged PR, either because contributors requested them,
  233. or because you found issues yourself while testing.
  234. The good news is that you can modify a pull request simply by acting on the
  235. branch you made the pull request from. You can e.g. make a new commit on that
  236. branch, push it to your fork, and the PR will be updated automatically:
  237. ::
  238. // Check out your branch again if you had changed in the meantime
  239. $ git checkout better-project-manager
  240. // Fix a mistake
  241. $ nano editor/project_manager.cpp
  242. $ git add editor/project_manager.cpp
  243. $ git commit -m "Fix a typo in the banner's title"
  244. $ git push origin better-project-manager
  245. That should do the trick, but...
  246. Mastering the PR workflow: the rebase
  247. -------------------------------------
  248. On the situation outlined above, your fellow contributors with an OCD
  249. regarding the Git history might ask your to *rebase* your branch to *squash*
  250. or *meld* the last two commits together (i.e. the two related to the project
  251. manager), as the second commit basically fixes an issue in the first one.
  252. Once the PR is merged, it is not relevant for a changelog reader that the PR
  253. author made mistakes; instead, we want to keep only commits that bring from
  254. one working state to another working state.
  255. To squash those two commits together, we will have to *rewrite history*.
  256. Right, we have that power. You may read that it's a bad practice, and it's
  257. true when it comes to branches of the upstream repo. But in your fork, you
  258. can do whatever you want, and everything is allowed to get neat PRs :)
  259. We will use the *interactive rebase* ``git rebase -i`` to do this. This
  260. command takes a commit hash as argument, and will let you modify all commits
  261. between that commit hash and the last one of the branch, the so-called
  262. *HEAD*. In our example, we want to act on the last two commits, so we will
  263. do:
  264. ::
  265. // The HEAD~X syntax means X commits before HEAD
  266. $ git rebase -i HEAD~2
  267. This will open a text editor with:
  268. ::
  269. pick 1b4aad7 Add a pretty banner to the project manager
  270. pick e07077e Fix a typo in the banner's title
  271. The editor will also show instructions regarding how you can act on those
  272. commits. In particular, it should tell you that "pick" means to use that
  273. commit (do nothing), and that "squash" and "fixup" can be used to *meld* the
  274. commit in its parent commit. The difference between "squash" and "fixup" is
  275. that "fixup" will discard the commit log from the squashed commit. In our
  276. example, we are not interested in keeping the log of the "Fix a typo" commit,
  277. so we use:
  278. ::
  279. pick 1b4aad7 Add a pretty banner to the project manager
  280. fixup e07077e Fix a typo in the banner's title
  281. Upon saving and quitting the editor, the rebase will occur. The second commit
  282. will be melded into the first one, and ``git log`` and ``git show`` should
  283. now confirm that you have only one commit with the changes from both previous
  284. commits.
  285. .. note:: You could have avoided this rebase by using ``git commit --amend``
  286. when fixing the typo. This command will write the staged changes
  287. directly into the *last* commit (*HEAD*), instead of creating a new
  288. commit like we did in this example. So it is equivalent to what we
  289. did with a new commit and then a rebase to mark it as "fixup".
  290. But! You rewrote the history, and now your local and remote branches have
  291. diverged. Indeed, commit 1b4aad7 in the above example will have changed, and
  292. therefore got a new commit hash. If you try to push to your remote branch, it
  293. will raise an error:
  294. ::
  295. $ git push origin better-project-manager
  296. To https://github.com/akien-mga/godot
  297. ! [rejected] better-project-manager -> better-project-manager (non-fast-forward)
  298. error: failed to push some refs to 'https://akien-mga@github.com/akien-mga/godot'
  299. hint: Updates were rejected because the tip of your current branch is behind
  300. hint: its remote counterpart.
  301. This is a sane behaviour, Git will not let you push changes that would
  302. override remote content. But that's actually what we want to do here, so we
  303. will have to *force* it:
  304. ::
  305. $ git push --force origin better-project-manager
  306. And tadaa! Git will happily *replace* your remote branch with what you had
  307. locally (so make sure that's what you wanted, using ``git log``). This will
  308. also update the PR accordingly.