]> git.apps.os.sepia.ceph.com Git - ceph.git/commitdiff
doc/dev/developer_guide: prefer Latinate verbs to English phrasal verbs 38144/head
authorAnthony D'Atri <anthony.datri@gmail.com>
Wed, 18 Nov 2020 01:04:57 +0000 (17:04 -0800)
committerAnthony D'Atri <anthony.datri@gmail.com>
Mon, 23 Nov 2020 21:07:19 +0000 (13:07 -0800)
Signed-off-by: Anthony D'Atri <anthony.datri@gmail.com>
Fixes: https://tracker.ceph.com/issues/46365
doc/dev/developer_guide/basic-workflow.rst

index 4a6913fb89a4df8aa0f0a24d8a88c7690717b343..1dfb0029d3d076bd4142c585573bdb83c0991642 100644 (file)
@@ -1,7 +1,7 @@
 Basic Workflow
 ==============
 
-The following chart illustrates basic development workflow:
+The following chart illustrates the basic Ceph development workflow:
 
 .. ditaa::
 
@@ -28,60 +28,62 @@ The following chart illustrates basic development workflow:
            | pull request |         git push       \-------------/
            \--------------/
 
-Below we present an explanation of this chart. The explanation is written
-with the assumption that you, the reader, are a beginning developer who
-has an idea for a bugfix, but do not know exactly how to proceed. Watch
-the `Getting Started with Ceph Development
-<https://www.youtube.com/watch?v=t5UIehZ1oLs>`_ video for
-a practical summary of the same.
+The below explanation is written with the assumption that you, the reader, are
+a new contributor who has an idea for a bugfix or enhancement, but do not know
+exactly how to proceed. Watch the `Getting Started with Ceph Development
+<https://www.youtube.com/watch?v=t5UIehZ1oLs>`_ video for a practical summary
+of this workflow.
 
 Update the tracker
 ------------------
 
-Before you start, you should know the :ref:`issue-tracker` number of the bug
-you intend to fix. If there is no tracker issue, now is the time to create
-one.
+Before you start, you should know the :ref:`issue-tracker` (Redmine) number
+of the bug you intend to fix. If there is no tracker issue, now is the time to
+create one for code changes.  Straightforward documentation cleanup does
+not necessarily require a corresponding tracker issue. However, an issue
+(ticket) should be created if one is adding new documentation chapters or
+files, or for other substantial changes.
 
-The tracker is there to explain the issue (bug) to your fellow Ceph
-developers and keep them informed as you make progress toward resolution.
-To this end, then, provide a descriptive title as well as sufficient
-information and details in the description.
+The tracker ticket serves to explain the issue (bug) to your fellow Ceph
+developers and keep them informed as you make progress toward resolution.  To
+this end, please provide a descriptive title and write appropriate information
+and details into the description.  When composing the ticket's title, consider "If I
+want to search for this ticket two years from now, what keywords will I search
+for?"
 
 If you have sufficient tracker permissions, assign the bug to yourself by
-changing the ``Assignee`` field.  If your tracker permissions have not yet
-been elevated, simply add a comment to the issue with a short message like
-"I am working on this issue".
+setting the ``Assignee`` field.  If your tracker permissions have not been
+elevated, simply add a comment with a short message like "I am working on this
+issue".
 
 Upstream code
 -------------
 
-This section, and the ones that follow, correspond to the nodes in the
-above chart.
+This section, and the ones that follow, correspond to nodes in the above chart.
 
-The upstream code lives in https://github.com/ceph/ceph.git, which is
-sometimes referred to as the "upstream repo", or simply "upstream". As the
-chart illustrates, we will make a local copy of this code, modify it, test
-our modifications, and submit the modifications back to the upstream repo
-for review.
+The upstream code is found at https://github.com/ceph/ceph.git, which is known
+as the "upstream repo", or simply "upstream". As the chart shows, we will make
+a local copy of this repository, modify it, test our modifications, then submit
+the modifications for review and merging.
 
 A local copy of the upstream code is made by
 
-1. forking the upstream repo on GitHub, and
-2. cloning your fork to make a local working copy
+1. Forking the upstream repo on GitHub, and
+2. Cloning your fork to make a local working copy
 
-See the `the GitHub documentation
+See the `GitHub documentation
 <https://help.github.com/articles/fork-a-repo/#platform-linux>`_ for
 detailed instructions on forking. In short, if your GitHub username is
-"mygithubaccount", your fork of the upstream repo will show up at
+"mygithubaccount", your fork of the upstream repo will appear at
 https://github.com/mygithubaccount/ceph. Once you have created your fork,
-you clone it by doing:
+clone it by running:
 
 .. prompt:: bash $
 
    git clone https://github.com/mygithubaccount/ceph
 
-While it is possible to clone the upstream repo directly, in this case you
-must fork it first. Forking is what enables us to open a `GitHub pull
+While it is possible to clone the upstream repo directly, for the Ceph workflow
+you must fork it first. Forking is what enables us to open a `GitHub pull
 request`_.
 
 For more information on using GitHub, refer to `GitHub Help
@@ -90,13 +92,25 @@ For more information on using GitHub, refer to `GitHub Help
 Local environment
 -----------------
 
-In the local environment created in the previous step, you now have a
-copy of the ``master`` branch in ``remotes/origin/master``. Since the fork
+In the local environment created in the previous step, you now have a copy of
+the ``master`` branch in ``remotes/origin/master``. This fork
 (https://github.com/mygithubaccount/ceph.git) is frozen in time and the
 upstream repo (https://github.com/ceph/ceph.git, typically abbreviated to
-``ceph/ceph.git``) is updated frequently by other developers, you will need
-to sync your fork periodically. To do this, first add the upstream repo as
-a "remote" and fetch it
+``ceph/ceph.git``) is updated frequently by other contributors, you must sync
+your fork periodically. Failure to do so may result in your commits and pull
+requests failing to merge because they refer to file contents that have since
+changed.
+
+First, ensure that you have properly configured your local git environment with
+your name and email address.  Skip this step if you have already configured this
+information.
+
+.. prompt:: bash $
+
+   git config user.name "FIRST_NAME LAST_NAME"
+   git config user.email "MY_NAME@example.com"
+
+Now add the upstream repo as a "remote" and fetch it:
 
 .. prompt:: bash $
 
@@ -107,10 +121,10 @@ Fetching downloads all objects (commits, branches) that were added since
 the last sync. After running these commands, all the branches from
 ``ceph/ceph.git`` are downloaded to the local git repo as
 ``remotes/ceph/$BRANCH_NAME`` and can be referenced as
-``ceph/$BRANCH_NAME`` in certain git commands.
+``ceph/$BRANCH_NAME`` in local git commands.
 
 For example, your local ``master`` branch can be reset to the upstream Ceph
-``master`` branch by doing
+``master`` branch by running
 
 .. prompt:: bash $
 
@@ -118,7 +132,7 @@ For example, your local ``master`` branch can be reset to the upstream Ceph
    git checkout master
    git reset --hard ceph/master
 
-Finally, the ``master`` branch of your fork can then be synced to upstream
+Finally, the ``master`` branch of your fork is synced to the upstream
 master by
 
 .. prompt:: bash $
@@ -128,7 +142,7 @@ master by
 Bugfix branch
 -------------
 
-Next, create a branch for the bugfix:
+Next, create a branch for your bugfix:
 
 .. prompt:: bash $
 
@@ -136,29 +150,31 @@ Next, create a branch for the bugfix:
    git checkout -b fix_1
    git push -u origin fix_1
 
-This creates a ``fix_1`` branch locally and in our GitHub fork. At this
-point, the ``fix_1`` branch is identical to the ``master`` branch, but not
-for long! You are now ready to modify the code.
+This creates a ``fix_1`` branch locally and in our GitHub fork. At this point,
+the ``fix_1`` branch is identical to the ``master`` branch, but not for long!
+You are now ready to modify the code.  Be careful to always run `git checkout
+master` first, otherwise you may find commits from an unrelated branch mixed
+with your new work.
 
 Fix bug locally
 ---------------
 
-At this point, change the status of the tracker issue to "In progress" to
-communicate to the other Ceph developers that you have begun working on a
-fix. If you don't have permission to change that field, your comment that
-you are working on the issue is sufficient.
+Now change the status of the tracker issue to "In progress" to communicate to
+other Ceph contributors that you have begun working on a fix.  This helps avoid
+duplication of effort.  If you don't have permission to change that field, your
+previous comment that you are working on the issue is sufficient.
 
-Possibly, your fix is very simple and requires only minimal testing.
-More likely, it will be an iterative process involving trial and error, not
-to mention skill. An explanation of how to fix bugs is beyond the
-scope of this document. Instead, we focus on the mechanics of the process
-in the context of the Ceph project.
+Your fix may be very simple and require only minimal testing.  More likely,
+this will be an iterative process involving trial and error, not to mention
+skill. An explanation of how to fix bugs is beyond the scope of this
+document. Instead, we focus on the mechanics of the process in the context of
+the Ceph project.
 
-A detailed discussion of the tools available for validating your bugfixes,
+For a detailed discussion of the tools available for validating bugfixes,
 see the chapters on testing.
 
-For now, let us just assume that you have finished work on the bugfix and
-that you have tested it and believe it works. Commit the changes to your local
+For now, let us just assume that you have finished work on the bugfix, that
+you have tested, and that you believe it works. Commit the changes to your local
 branch using the ``--signoff`` option
 
 .. prompt:: bash $
@@ -174,44 +190,48 @@ and push the changes to your fork
 GitHub pull request
 -------------------
 
-The next step is to open a GitHub pull request. The purpose of this step is
-to make your bugfix available to the community of Ceph developers.  They
-will review it and may do additional testing on it.
+The next step is to open a GitHub pull request (PR). This makes your bugfix
+visible to the community of Ceph contributors.  They will review it and may
+perform additional testing and / or request changes.
 
-In short, this is the point where you "go public" with your modifications.
-Psychologically, you should be prepared to receive suggestions and
-constructive criticism. Don't worry! In our experience, the Ceph project is
-a friendly place!
+This is the point where you "go public" with your modifications.  Be prepared
+to receive suggestions and constructive criticism in the form of comments
+within the PR. Don't worry!  The Ceph project is a friendly place!
 
-If you are uncertain how to use pull requests, you may read
+If you are uncertain how to create and manage pull requests, you may read
 `this GitHub pull request tutorial`_.
 
 .. _`this GitHub pull request tutorial`:
    https://help.github.com/articles/using-pull-requests/
 
-For some ideas on what constitutes a "good" pull request, see
+For ideas on what constitutes a "good" pull request, see
 the `Git Commit Good Practice`_ article at the `OpenStack Project Wiki`_.
 
 .. _`Git Commit Good Practice`: https://wiki.openstack.org/wiki/GitCommitMessages
 .. _`OpenStack Project Wiki`: https://wiki.openstack.org/wiki/Main_Page
 
+and our own `Submitting Patches <https://github.com/ceph/ceph/blob/master/SubmittingPatches.rst>`_ document.
+
 Once your pull request (PR) is opened, update the :ref:`issue-tracker` by
-adding a comment to the bug pointing the other developers to your PR. The
-update can be as simple as::
+adding a comment directing other contributors to your PR. The comment can be
+as simple as::
 
     *PR*: https://github.com/ceph/ceph/pull/$NUMBER_OF_YOUR_PULL_REQUEST
 
 Automated PR validation
 -----------------------
 
-When your PR hits GitHub, the Ceph project's `Continuous Integration (CI)
-<https://en.wikipedia.org/wiki/Continuous_integration>`_
-infrastructure will test it automatically. At the time of this writing
-(September 2020), the automated CI testing included five tests to check that the
-commits in the PR are properly signed (see :ref:`submitting-patches`), to check that the documentation builds, to check that the submodules are unmodified, to check that the API is in order,  and a :ref:`make-check` test.
+When your PR is created or updated, the Ceph project's `Continuous Integration
+(CI) <https://en.wikipedia.org/wiki/Continuous_integration>`_ infrastructure
+will test it automatically. At the time of this writing (September 2020), the
+automated CI testing included five tests to check that the commits in the PR
+are properly signed (see :ref:`submitting-patches`), to check that the
+documentation builds, to check that the submodules are unmodified, to check
+that the API is in order, and a :ref:`make-check` test.  Additional tests may
+be performed depending on which files are modified by your PR.
 
 The :ref:`make-check`, builds the PR and runs it through a battery of
-tests. These tests run on machines operated by the Ceph Continuous
+tests. These tests run on servers operated by the Ceph Continuous
 Integration (CI) team. When the tests complete, the result will be shown
 on GitHub in the pull request itself.
 
@@ -224,25 +244,32 @@ Notes on PR make check test
 The GitHub :ref:`make check<make-check>` test is driven by a Jenkins instance.
 
 Jenkins merges your PR branch into the latest version of the base branch before
-starting the build. This means that you don't have to rebase the PR to pick up any fixes.
+starting tests. This means that you don't have to rebase the PR to pick up any fixes.
 
-You can trigger the PR tests at any time by adding a comment to the PR - the
+You can trigger PR tests at any time by adding a comment to the PR - the
 comment should contain the string "test this please". Since a human subscribed
-to the PR might interpret that as a request for him or her to test the PR,
-we recommend that you address Jenkins directly in the request; for example, write "jenkins retest this please".
-
-If there is a build failure and you aren't sure what caused it, check the :ref:`make check<make-check>` log. To access it, first click on "details" (next
-to the :ref:`make check<make-check>` test in the PR) to enter the Jenkins
-web GUI. Then click on "Console Output" (on the left).
-
-Jenkins is set up to grep the log for strings known to have been associated
-with :ref:`make check<make-check>` failures in the past. However, there is no guarantee that the known strings are associated with any given :ref:`make check<make-check>` failure. You'll have to dig into the log to determine the cause of the failure.
+to the PR might interpret that as a request for him or her to test the PR, we
+recommend that you address Jenkins directly. For example, write "jenkins retest
+this please".  For efficiency a single re-test can also be requested with
+e.g. "jenkins test signed".  For reference, a list of these requests is
+automatically added to the end of each new PR's description.
+
+If there is a build failure and you aren't sure what caused it, check the
+:ref:`make check<make-check>` log. To access it, click on the "details" (next
+to the :ref:`make check<make-check>` test in the PR) link to enter the Jenkins web
+GUI. Then click on "Console Output" (on the left).
+
+Jenkins is configured to search logs for strings known to have been associated
+with :ref:`make check<make-check>` failures in the past. However, there is no
+guarantee that these known strings are associated with any given
+:ref:`make check<make-check>` failure. You'll have to read through the log to determine the
+cause of your specific failure.
 
 Integration tests AKA ceph-qa-suite
 -----------------------------------
 
-Since Ceph is a complex beast, it may also be necessary to test your fix to
-see how it behaves on real clusters running either on real or virtual
+Since Ceph is complex, it may be necessary to test your fix to
+see how it behaves on real clusters running on physical or virtual
 hardware. Tests designed for this purpose live in the `ceph/qa
 sub-directory`_ and are run via the `teuthology framework`_.
 
@@ -252,7 +279,7 @@ sub-directory`_ and are run via the `teuthology framework`_.
 
 The Ceph community has access to the `Sepia lab
 <https://wiki.sepia.ceph.com/doku.php>`_ where :ref:`testing-integration-tests` can be
-run on real hardware. Other developers may add tags like "needs-qa" to your
+run on physical hardware. Other developers may add tags like "needs-qa" to your
 PR. This allows PRs that need testing to be merged into a single branch and
 tested all at the same time. Since teuthology suites can take hours (even
 days in some cases) to run, this can save a lot of time.
@@ -267,7 +294,7 @@ Code review
 
 Once your bugfix has been thoroughly tested, or even during this process,
 it will be subjected to code review by other developers. This typically
-takes the form of correspondence in the PR itself, but can be supplemented
+takes the form of comments in the PR itself, but can be supplemented
 by discussions on :ref:`irc` and the :ref:`mailing-list`.
 
 Amending your PR
@@ -276,7 +303,7 @@ Amending your PR
 While your PR is going through testing and `Code Review`_, you can
 modify it at any time by editing files in your local branch.
 
-After the changes are committed locally (to the ``fix_1`` branch in our
+After updates are committed locally (to the ``fix_1`` branch in our
 example), they need to be pushed to GitHub so they appear in the PR.
 
 Modifying the PR is done by adding commits to the ``fix_1`` branch upon
@@ -290,11 +317,16 @@ will need to force push your branch with:
 
    git push --force origin fix_1
 
+Why do we take these extra steps instead of simply adding additional commits
+the the PR?  It is best practice for a PR to consist of a single commit; this
+makes for clean history, eases peer review of your changes, and facilitates
+merges.  In rare circumstances it also makes it easier to cleanly revert
+changes.
+
 Merge
 -----
 
-The bugfixing process culminates when one of the project leads decides to
-merge your PR.
+The bugfix process completes when a project lead merges your PR.
 
 When this happens, it is a signal for you (or the lead who merged the PR)
 to change the :ref:`issue-tracker` status to "Resolved". Some issues may be
@@ -314,22 +346,22 @@ This is the most basic form of a merge commit::
 
 This consists of two parts:
 
-#. The title of the commit of the pull request to be merged.
+#. The title of the commit / PR to be merged.
 #. The name and email address of the reviewer. Enclose the reviewer's email 
    address in angle brackets.
 
 Using .githubmap to Find a Reviewer's Email Address
 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
-If you cannot find the email address of the reviewer on his or her github
+If you cannot find the email address of the reviewer on his or her GitHub
 page, you can look it up in the **.githubmap** file, which can be found in
 the repository at **/ceph/.githubmap**.
 
 Using "git log" to find a Reviewer's Email Address
 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
-If you cannot find a reviewer's email address by using the above methods,
-you can search the git log for their email address. Reviewers are likely
-to have committed something before, and as long as they have committed
-something, the git log will probably contain their email address.
+If you cannot find a reviewer's email address by using the above methods, you
+can search the git log for their email address. Reviewers are likely to have
+committed something before.  If they have made previous contributions, the git
+log will probably contain their email address.
 
 Use the following command
 
@@ -350,3 +382,4 @@ the **ptl-tool** have the following form::
              client: move client_lock to _unmount()
              client: add timer_lock support
      Reviewed-by: Patrick Donnelly <pdonnell@redhat.com>
+