Browse Source

Add contributing docs on the PR workflow

(cherry picked from commit 84538955e04bd0354b1ef869d8edd4d49d0b28e6)
Rémi Verschelde 8 years ago
parent
commit
1ea4364765

+ 1 - 0
community/contributing/index.rst

@@ -6,6 +6,7 @@ Contributing
    :name: toc-community-contributing
    :name: toc-community-contributing
 
 
    ways_to_contribute
    ways_to_contribute
+   pr_workflow
    bug_triage_guidelines
    bug_triage_guidelines
    doc_and_l10n_guidelines
    doc_and_l10n_guidelines
    updating_the_class_reference
    updating_the_class_reference

+ 409 - 0
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 <https://github.com/godotengine/godot>``_ is a
+``Git <https://git-scm.com>``_ 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 <https://git-scm.com/book/en/v2>``_ 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 <https://github.com/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 -- <files>`` will undo the unstaged changes to the given
+  files.
+- ``git add <files>`` 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 <files>`` 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 <remote> <local branch>[:<remote branch>]
+
+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://[email protected]/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.

+ 4 - 4
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
 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.
 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
 Testing and reporting issues
 ----------------------------
 ----------------------------
@@ -171,7 +171,7 @@ There are two separate resources referred to as "documentation" in Godot:
    :ref:`Godot API <toc-class-ref>`.
    :ref:`Godot API <toc-class-ref>`.
    To contribute to the class reference, you have to edit the
    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.
    `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
 -  **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
    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
    to which you can contribute via pull requests on the
    `godot-docs <https://github.com/godotengine/godot-docs>`_ GitHub repository.
    `godot-docs <https://github.com/godotengine/godot-docs>`_ GitHub repository.
 
 
-.. TODO. See :ref:`documentation_guidelines` for more details.
+.. TODO. See :ref:`doc_documentation_guidelines` for more details.

BIN
img/github_fork_button.png


BIN
img/github_fork_make_pr.png


BIN
img/github_fork_url.png