diff --git a/community/contributing/index.rst b/community/contributing/index.rst index c00c5728f..1e24a4790 100644 --- a/community/contributing/index.rst +++ b/community/contributing/index.rst @@ -6,6 +6,7 @@ Contributing :name: toc-community-contributing ways_to_contribute + pr_workflow bug_triage_guidelines doc_and_l10n_guidelines updating_the_class_reference diff --git a/community/contributing/pr_workflow.rst b/community/contributing/pr_workflow.rst new file mode 100644 index 000000000..5c15a01e8 --- /dev/null +++ b/community/contributing/pr_workflow.rst @@ -0,0 +1,409 @@ +.. _doc_pr_workflow: + +Pull request workflow +===================== + +.. highlight:: shell + +The so-called "PR workflow" used by Godot is common to many projects using +Git, and should be familiar to veteran free software contributors. The idea +is that only a small number (if any) commit directly to the *master* branch. +Instead, contributors *fork* the project (i.e. create a copy of it, which +they can modify as they wish), and then use the GitHub interface to request +a *pull* from one of their fork's branch to one branch of the original +(often named *upstream*) repository. + +The resulting *pull request* (PR) can then be reviewed by other contributors, +which might approve it, reject it, or most often request that modifications +be done. Once approved, the PR can then be merged by one of the core +developers, and its commit(s) will become part of the target branch (usually +the *master* branch). + +We will go together through an example to show the typical workflow and +associated Git commands. But first, let's have a quick look at the +organisation of Godot's Git repository. + +Git source repository +--------------------- + +The ``repository on GitHub ``_ is a +``Git ``_ code repository together with an embedded +issue tracker and PR system. + +The Git version control system is the tool used to keep track of successive +edits to the source code - to contibute efficiently to Godot, learning the +basics of the Git command line is *highly* recommended. There exist some +graphical interfaces for Git, but they usually encourage users to take bad +habits regarding the Git and PR workflow, and we therefore recommend not to +use them (especially GitHub's online editor). + +.. seealso:: The first sections of Git's "Book" are a good introduction to + the tool's philosophy and the various commands you need to + master in your daily workflow. You can read them online on the + ``Git SCM ``_ website. + +The branches on the Git repository are organized as follows: + +- The *master* branch is where the development of the next major version + (3.0, 3.1, 4.0, etc.) occurs. As a development branch, it can be unstable + and is not meant for use in production. This is where PRs should be done + in priority. +- The stable branches are named after their version, e.g. *2.0* and *2.1*. + They are used to backport bugfixes and enhancements from the *master* + branch to the currently maintained stable release (e.g. 2.0.1 or 2.1.3). + As a rule of thumb, the last stable branch is maintained until the next + major version (e.g. the *2.0* branch was maintained until the release of + Godot 2.1). + If you want to make PRs against a maintained stable branch, you will have + to check if your changes are also relevant for the *master* branch. +- There might be feature branches at time, usually meant to be merged into + the *master* branch at some time. + +Forking and cloning +------------------- + +The first step is to *fork* the ``godotengine/godot ``_ +repository on GitHub. To do so, you will need to have a GitHub account and to +be logged in. In the top right corner of the repository's GitHub page, you +should see the "Fork" button as shown below: + +.. image:: /img/github_fork_button.png + +Click it, and after a while you should be redirected to your own fork of the +Godot repo, with your GitHub username as namespace: + +.. image:: /img/github_fork_url.png + +You can then *clone* your fork, i.e. create a local copy of the online +repository (in Git speak, the *origin remote*): + +:: + + $ git clone https://github.com/USERNAME/godot + +.. note:: In our examples, the "$" character denotes the command line prompt + on typical UNIX shells. It is not part of the command and should + not be typed. + +After a little while, you should have a ``godot`` directory in your current +working directory. Move into it (``cd godot``), and we will set up a useful +reference: + +:: + + $ git remote add upstream https://github.com/godotengine/godot + $ git fetch upstream + +This will create a reference named *upstream* pointing to the original +godotengine/godot repository. This will be useful when you want to pull new +commits from its *master* branch to update your fork. You have another +*remote* reference named *origin*, which points to your fork. + +You only need to do the above steps once, as long as you keep that local +``godot`` folder (which you can move around if you want, the relevant +metadata is hidden in its ``.git`` subfolder). + +.. note:: *Branch it, pull it, code it, stage it, commit, push it, rebase + it... technologic.* + + This bad take on Daft Punk's *Technologic* shows the general + conception Git beginners have of its workflow: lots of strange + commands to learn by copy and paste, hoping they will work as + expected. And that's actually not a bad way to learn, as long as + you're curious and don't hesitate to question your search engine + when lost, so we will give you the basic commands to know when + working in Git. + +In the following, we will assume that you want to implement a feature in +Godot's project manager, which is coded in the ``editor/project_manager.cpp`` +file. + +Branching +--------- + +By default, the ``git clone`` should have put you on the *master* branch of +your fork (*origin*). To start your own feature development, we will create +a feature branch: + +:: + + // Create the branch based on the current branch (master) + $ git branch better-project-manager + // Change the current branch to the new one + $ git checkout better-project-manager + +This command is equivalent: + +:: + + // Change the current branch to a new named one, based on the current branch + $ git checkout -b better-project-manager + +If you want to go back to the *master* branch, you'd use: + +:: + + $ git checkout master + +You can see which branch you are currently on with the ``git branch`` +command: + +:: + + $ git branch + 2.1 + * better-project-manager + master + +Updating your branch +-------------------- + +This would not be needed the first time, just after you forked the upstream +repository. However, the next time you want to work on something, you will +notice that your fork's *master* is several commits behind the upstream +*master* branch: pull requests from other contributors would have been merged +in the meantime. + +To ensure there won't be conflicts between the feature you develop and the +current upstream *master* branch, you will have to update your branch by +*pulling* the upstream branch. + +:: + + $ git pull upstream master + +However, if you had local commits, this method will create a so-called "merge +commit", and you will soon hear from fellow contributors that those are not +wanted in PRs. Then how to update the branch without creating a merge commit? +You will have to use the ``--rebase`` option, so that your local commits are +replayed on top of the updated upstream *master* branch. It will effectively +modify the Git history of your branch, but that is for the greater good. + +Then command that you should (almost) always use is there: + +:: + + $ git pull --rebase upstream master + +Making changes +-------------- + +You would then do your changes to our example's +``editor/project_manager.cpp`` file with your usual development environment +(text editor, IDE, etc.). + +By default, those changes are *unstaged*. The staging area is a layer between +your working directory (where you make your modifications) and the local git +repository (the commits and all the metadata in the ``.git`` folder). To +bring changes from the working directory to the git repository, you need to +*stage* them with the ``git add`` command, and then to commit them with the +``git commit`` command. + +There are various commands you should know to review your current work, +before staging it, while it is staged, and after it has been committed. + +- ``git diff`` will show you the current unstaged changes, i.e. the + differences between your working directory and the staging area. +- ``git checkout -- `` will undo the unstaged changes to the given + files. +- ``git add `` will *stage* the changes on the listed files. +- ``git diff --staged`` will show the current staged changes, i.e. the + differences between the staging area and the last commit. +- ``git reset HEAD `` will *unstage* changes to the listed files. +- ``git status`` will show you what are the currently staged and unstaged + modifications. +- ``git commit`` will commit the staged files. It will open a text editor + (you can define the one you want to use with the ``GIT_EDITOR`` environment + variable or the ``core.editor`` setting in your Git config) to let you + write a commit log. You can use ``git commit -m "Cool commit log"`` to + write the log directly. +- ``git log`` will show you the last commits of your current branch. If you + did local commits, they should be shown at the top. +- ``git show`` will show you the changes of the last commit. You can also + specify a commit hash to see the changes for that commit. + +That's a lot to memorise! Don't worry, just check this cheat sheet when you +need to make changes, and learn by doing. + +Here's how the shell history could look like on our example: + +:: + + // It's nice to know where you're starting from + $ git log + // Do changes to the project manager + $ nano editor/project_manager.cpp + // Find an unrelated bug in Control and fix it + $ nano scene/gui/control.cpp + // Review changes + $ git status + $ git diff + // We'll do two commits for our unrelated changes, + // starting by the Control changes necessary for the PM enhancements + $ git add scene/gui/control.cpp + $ git commit -m "Fix handling of margins in Control" + // Check we did good + $ git log + $ git show + $ git status + // Make our second commit + $ git add editor/project_manager.cpp + $ git commit -m "Add a pretty banner to the project manager" + $ git log + +With this, we should have two new commits in our *better-project-manager* +branch which were not in the *master* branch. They are still only local +though, the remote fork does not know about them, nor does the upstream repo. + +Pushing changes to a remote +--------------------------- + +That's where ``git push`` will come into play. In Git, a commit is always +done in the local repository (unlike Subversion where a commit will modify +the remote repository directly). You need to *push* the new commits to a +remote branch to share them with the world. The syntax for this is: + +:: + + $ git push [:] + +The part about the remote branch can be ommitted if you want it to have the +same name as the local branch, which is our case in this example, so we will +do: + +:: + + $ git push origin better-project-manager + +Git will ask you for your username and password, and the changes will be sent +to your remote. If you check the fork's page on GitHub, you should see a new +branch with your added commits. + +Issuing a pull request +---------------------- + +When you load your fork's branch on GitHub, you should see a line saying +"This branch is 2 commits ahead of godotengine:master." (and potentially some +commits behind, if your *master* branch was out of sync with the upstream +*master* branch. + +.. image:: /img/github_fork_make_pr.png + +On that line, there is a "Pull request" link. Clicking it will open a form +that will let you issue a pull request on the godotengine/godot upstream +repository. It should show you your two commits, and state "Able to merge". +If not (e.g. it has way more commits, or says there are merge conflicts), +don't create the PR, something went wrong. Go to IRC and ask for support :) + +Use an explicit title for the PR and put the necessary details in the comment +area. You can drag and drop screenshots, gifs or zipped projects if relevant, +to showcase what your work implements. Click "Create a pull request", and +tadaa! + +Modifying a pull request +------------------------ + +While it is reviewed by other contributors, you will often need to make +changes to your yet-unmerged PR, either because contributors requested them, +or because you found issues yourself while testing. + +The good news is that you can modify a pull request simply by acting on the +branch you made the pull request from. You can e.g. make a new commit on that +branch, push it to your fork, and the PR will be updated automatically: + +:: + + // Check out your branch again if you had changed in the meantime + $ git checkout better-project-manager + // Fix a mistake + $ nano editor/project_manager.cpp + $ git add editor/project_manager.cpp + $ git commit -m "Fix a typo in the banner's title" + $ git push origin better-project-manager + +That should do the trick, but... + +Mastering the PR workflow: the rebase +------------------------------------- + +On the situation outlined above, your fellow contributors with an OCD +regarding the Git history might ask your to *rebase* your branch to *squash* +or *meld* the last two commits together (i.e. the two related to the project +manager), as the second commit basically fixes an issue in the first one. + +Once the PR is merged, it is not relevant for a changelog reader that the PR +author made mistakes; instead, we want to keep only commits that bring from +one working state to another working state. + +To squash those two commits together, we will have to *rewrite history*. +Right, we have that power. You may read that it's a bad practice, and it's +true when it comes to branches of the upstream repo. But in your fork, you +can do whatever you want, and everything is allowed to get neat PRs :) + +We will use the *interactive rebase* ``git rebase -i`` to do this. This +command takes a commit hash as argument, and will let you modify all commits +between that commit hash and the last one of the branch, the so-called +*HEAD*. In our example, we want to act on the last two commits, so we will +do: + +:: + + // The HEAD~X syntax means X commits before HEAD + $ git rebase -i HEAD~2 + +This will open a text editor with: + +:: + + pick 1b4aad7 Add a pretty banner to the project manager + pick e07077e Fix a typo in the banner's title + +The editor will also show instructions regarding how you can act on those +commits. In particular, it should tell you that "pick" means to use that +commit (do nothing), and that "squash" and "fixup" can be used to *meld* the +commit in its parent commit. The difference between "squash" and "fixup" is +that "fixup" will discard the commit log from the squashed commit. In our +example, we are not interested in keeping the log of the "Fix a typo" commit, +so we use: + +:: + + pick 1b4aad7 Add a pretty banner to the project manager + fixup e07077e Fix a typo in the banner's title + +Upon saving and quitting the editor, the rebase will occur. The second commit +will be melded into the first one, and ``git log`` and ``git show`` should +now confirm that you have only one commit with the changes from both previous +commits. + +.. note:: You could have avoided this rebase by using ``git commit --amend`` + when fixing the typo. This command will write the staged changes + directly into the *last* commit (*HEAD*), instead of creating a new + commit like we did in this example. So it is equivalent to what we + did with a new commit and then a rebase to mark it as "fixup". + +But! You rewrote the history, and now your local and remote branches have +diverged. Indeed, commit 1b4aad7 in the above example will have changed, and +therefore got a new commit hash. If you try to push to your remote branch, it +will raise an error: + +:: + + $ git push origin better-project-manager + To https://github.com/akien-mga/godot + ! [rejected] better-project-manager -> better-project-manager (non-fast-forward) + error: failed to push some refs to 'https://akien-mga@github.com/akien-mga/godot' + hint: Updates were rejected because the tip of your current branch is behind + hint: its remote counterpart. + +This is a sane behaviour, Git will not let you push changes that would +override remote content. But that's actually what we want to do here, so we +will have to *force* it: + +:: + + $ git push --force origin better-project-manager + +And tadaa! Git will happily *replace* your remote branch with what you had +locally (so make sure that's what you wanted, using ``git log``). This will +also update the PR accordingly. diff --git a/community/contributing/ways_to_contribute.rst b/community/contributing/ways_to_contribute.rst index dfc035d0d..0a878c125 100644 --- a/community/contributing/ways_to_contribute.rst +++ b/community/contributing/ways_to_contribute.rst @@ -82,8 +82,8 @@ To ensure a good collaboration and overall quality, the Godot developers enforce some rules for code contribution, for example regarding the style to use in the C++ code (indentation, brackets, etc.) or the git and PR workflow. -.. TODO. Those technical details are outlined in a specific section, - :ref:`doc_code_contribution_guidelines`. +.. seealso:: Technical details about the PR workflow are outlined in a + specific section, :ref:`doc_pr_workflow`. Testing and reporting issues ---------------------------- @@ -171,7 +171,7 @@ There are two separate resources referred to as "documentation" in Godot: :ref:`Godot API `. To contribute to the class reference, you have to edit the `doc/base/classes.xml` in Godot's git repository, and make a pull request. - See :ref:`updating_the_class_reference` for more details. + See :ref:`doc_updating_the_class_reference` for more details. - **The tutorials and engine documentation.** This is the part you are reading now, which is distributed in the HTML, PDF and EPUB formats. Its contents @@ -179,4 +179,4 @@ There are two separate resources referred to as "documentation" in Godot: to which you can contribute via pull requests on the `godot-docs `_ GitHub repository. -.. TODO. See :ref:`documentation_guidelines` for more details. +.. TODO. See :ref:`doc_documentation_guidelines` for more details. diff --git a/img/github_fork_button.png b/img/github_fork_button.png new file mode 100644 index 000000000..3b3d9a574 Binary files /dev/null and b/img/github_fork_button.png differ diff --git a/img/github_fork_make_pr.png b/img/github_fork_make_pr.png new file mode 100644 index 000000000..6c04ca4e2 Binary files /dev/null and b/img/github_fork_make_pr.png differ diff --git a/img/github_fork_url.png b/img/github_fork_url.png new file mode 100644 index 000000000..154ece52f Binary files /dev/null and b/img/github_fork_url.png differ