1 ==========================
2 Submitting Patches to Ceph
3 ==========================
5 Patches to Ceph can be divided into three categories:
7 1. patches targeting Ceph kernel code
8 2. patches targeting the "master" branch
9 3. patches targeting stable branches (e.g.: "nautilus")
11 Some parts of Ceph - notably the RBD and CephFS kernel clients - are maintained
12 within the Linux Kernel. For patches targeting this code, please refer to the
13 file ``SubmittingPatches-kernel.rst``.
15 The rest of this document assumes that your patch relates to Ceph code that is
16 maintained in the GitHub repository https://github.com/ceph/ceph
18 If you have a patch that fixes an issue, feel free to open a GitHub pull request
19 ("PR") targeting the "master" branch, but do read this document first, as it
20 contains important information for ensuring that your PR passes code review
23 For patches targeting stable branches (e.g. "nautilus"), please also see
24 the file ``SubmittingPatches-backports.rst``.
33 The sign-off is a simple line at the end of the explanation for the
34 commit, which certifies that you wrote it or otherwise have the right to
35 pass it on as a open-source patch. The rules are pretty simple: if you
36 can certify the below:
38 Developer's Certificate of Origin 1.1
39 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
41 By making a contribution to this project, I certify that:
43 (a) The contribution was created in whole or in part by me and I
44 have the right to submit it under the open source license
45 indicated in the file; or
47 (b) The contribution is based upon previous work that, to the best
48 of my knowledge, is covered under an appropriate open source
49 license and I have the right under that license to submit that
50 work with modifications, whether created in whole or in part
51 by me, under the same open source license (unless I am
52 permitted to submit under a different license), as indicated
55 (c) The contribution was provided directly to me by some other
56 person who certified (a), (b) or (c) and I have not modified
59 (d) I understand and agree that this project and the contribution
60 are public and that a record of the contribution (including all
61 personal information I submit with it, including my sign-off) is
62 maintained indefinitely and may be redistributed consistent with
63 this project or the open source license(s) involved.
65 then you just add a line saying ::
67 Signed-off-by: Random J Developer <random@developer.example.org>
69 using your real name (sorry, no pseudonyms or anonymous contributions).
71 Git can sign off on your behalf
72 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
74 Please note that git makes it trivially easy to sign commits. First, set the
75 following config options::
77 $ git config --list | grep user
78 user.email=my_real_email_address@example.com
79 user.name=My Real Name
81 Then just remember to use ``git commit -s``. Git will add the ``Signed-off-by``
88 Group *logical changes* into individual commits.
90 If you have a series of bulleted modifications, consider separating each of
91 those into its own commit.
93 For example, if your changes include both bug fixes and performance enhancements
94 for a single component, separate those changes into two or more commits. If your
95 changes include an API update, and a new feature which uses that new API,
96 separate those into two patches.
98 On the other hand, if you make a single change that affects numerous
99 files, group those changes into a single commit. Thus a single logical change is
100 contained within a single patch. (If the change needs to be backported, that
101 might change the calculus, because smaller commits are easier to backport.)
104 Describe your changes
105 ---------------------
107 Each commit has an associated commit message that is stored in git. The first
108 line of the commit message is the `commit title`_. The second line should be
109 left blank. The lines that follow constitute the `commit message`_.
111 A commit and its message should be focused around a particular change.
116 The text up to the first empty line in a commit message is the commit
117 title. It should be a single short line of at most 72 characters,
118 summarizing the change, and prefixed with the
119 subsystem or module you are changing. Also, it is conventional to use the
120 imperative mood in the commit title. Positive examples include::
122 mds: add perf counter for finisher of MDSRank
123 osd: make the ClassHandler::mutex private
125 If the change only touches the files under ``doc`` directory, the title
126 should start with "doc". For instance, a commit fixing a typo in
127 ``doc/mgr/dashboard.rst`` could have a title like::
129 doc/mgr/dashboard: fix a typo
131 More positive examples can be obtained from the git history of the ``master``
136 Some negative examples (how *not* to title a commit message)::
142 Further to the last negative example ("fix issue 99999"), see `Fixes line(s)`_.
147 (This section is about the body of the commit message. Please also see
148 the preceding section, `Commit title`_, for advice on titling commit messages.)
150 In the body of your commit message, be as specific as possible. If the commit
151 message title was too short to fully state what the commit is doing, use the
152 body to explain not just the "what", but also the "why".
154 For positive examples, peruse ``git log`` in the ``master`` branch. A negative
155 example would be a commit message that merely states the obvious. For example:
156 "this patch includes updates for subsystem X. Please apply."
161 If the commit fixes one or more issues tracked by http://tracker.ceph.com,
162 add a ``Fixes:`` line (or lines) to the commit message, to connect this change
163 to addressed issue(s) - for example::
165 Fixes: http://tracker.ceph.com/issues/12345
167 This line should be added just before the ``Signed-off-by:`` line (see `Sign
170 It helps reviewers to get more context of this bug and facilitates updating of
171 the bug tracker. Also, anyone perusing the git history will see this line and be
172 able to refer to the bug tracker easily.
174 Here is an example showing a properly-formed commit message::
176 doc: add "--foo" option to bar
178 This commit updates the man page for bar with the newly added "--foo"
181 Fixes: http://tracker.ceph.com/issues/12345
182 Signed-off-by: Random J Developer <random@developer.example.org>
184 If a commit fixes a regression introduced by a different commit, please also
185 (in addition to the above) add a line referencing the SHA1 of the commit that
186 introduced the regression. For example::
188 Fixes: 9dbe7a003989f8bb45fe14aaa587e9d60a392727
194 PRs should be opened on branches contained in your fork of
195 https://github.com/ceph/ceph.git - do not push branches directly to
198 PRs should target "master". If you need to add a patch to a stable branch, such
199 as "nautilus", see the file ``SubmittingPatches-backports.rst``.
201 In addition to a base, or "target" branch, PRs have several other components:
202 the `PR title`_, the `PR description`_, labels, comments, etc. Of these, the PR
203 title and description are relevant for new contributors.
208 If your PR has only one commit, the PR title can be the same as the commit title
209 (and GitHub will suggest this). If the PR has multiple commits, do not accept
210 the title GitHub suggests. Either use the title of the most relevant commit, or
211 write your own title. In the latter case, use the same "subsystem: short
212 description" convention described in `Commit title`_ for the PR title, with
213 the following difference: the PR title describes the entire set of changes,
214 while the `Commit title`_ describes only the changes in a particular commit.
216 If GitHub suggests a PR title based on a very long commit message it will split
217 the result with an elipsis (...) and fold the remainder into the PR description.
218 In such a case, please edit the title to be more concise and the description to
221 Keep in mind that the PR titles feed directly into the script that generates
222 release notes and it is tedious to clean up non-conformant PR titles at release
223 time. This document places no limit on the length of PR titles, but be aware
224 that they are subject to editing as part of the release process.
229 In addition to a title, the PR also has a description field, or "body".
231 The PR description is a place for summarizing the PR as a whole. It need not
232 duplicate information that is already in the commit messages. It can contain
233 notices to maintainers, links to tracker issues and other related information,
234 to-do lists, etc. The PR title and description should give readers a high-level
235 notion of what the PR is about, quickly enabling them to decide whether they
236 should take a closer look.
239 Flag your changes for backport
240 ------------------------------
242 If you believe your changes should be backported to stable branches after the PR
243 is merged, open a tracker issue at https://tracker.ceph.com explaining:
246 2. why does the bug need to be fixed in <release>
248 and fill out the Backport field in the tracker issue. For example::
250 Backport: mimic, nautilus
252 For information on how backports are done in the Ceph project, refer to the
253 document ``SubmittingPatches-backports.rst``.
259 Before opening your PR, it's a good idea to run tests on your patchset. Doing
260 that is simple, though the process can take a long time to complete, especially
261 on older machines with less memory and spinning disks.
263 The most simple test is to verify that your patchset builds, at least in your
264 own development environment. The commands for this are::
270 Ceph comes with a battery of tests that can be run on a single machine. These
271 are collectively referred to as "make check", and can be run by executing the
276 If your patchset does not build, or if one or more of the "make check" tests
277 fails, but the error shown is not obviously related to your patchset, don't let
278 that dissuade you from opening a PR. The Ceph project has a Jenkins instance
279 which will build your PR branch and run "make check" on it in a controlled
282 Once your patchset builds and passes "make check", you can run even more tests
283 on it by issuing the following commands::
286 ../qa/run-standalone.sh
288 Like "make check", the standalone tests take a long time to run. They also
289 produce voluminous output. If one or more of the standalone tests fails, it's
290 likely the relevant part of the output will have scrolled off your screen or
291 gotten swapped out of your buffer. Therefore, it makes sense to capture the
292 output in a file for later analysis.
295 Document your changes
296 ---------------------
298 If you have added or modified any user-facing functionality, such as CLI
299 commands or their output, then the pull request must include appropriate updates
302 It is the submitter's responsibility to make the changes, and the reviewer's
303 responsibility to make sure they are not merging changes that do not
304 have the needed updates to documentation.
306 Where there are areas that have absent documentation, or there is no clear place
307 to note the change that is being made, the reviewer should contact the component
308 lead, who should arrange for the missing section to be created with sufficient
309 detail for the PR submitter to document their changes.
311 When writing and/or editing documentation, follow the Google Developer
312 Documentation Style Guide: https://developers.google.com/style/