Difference between revisions of "Starting as a Contributor"
m (→Existing tools: fix broken links) |
Adamdruppe (talk | contribs) (feep tells me this is more sane cuz it avoids deleting someone else's data) |
||
(59 intermediate revisions by 8 users not shown) | |||
Line 1: | Line 1: | ||
This page describes how to build D, put together a correct patch, and contribute it as a GitHub pull request. | This page describes how to build D, put together a correct patch, and contribute it as a GitHub pull request. | ||
− | |||
− | |||
− | |||
− | |||
==Existing tools== | ==Existing tools== | ||
− | + | If you only want to build the latest and greatest DMD, there exist tools which can do this automatically: | |
− | |||
− | |||
− | |||
− | |||
− | |||
− | |||
− | |||
− | |||
− | |||
− | |||
− | |||
− | |||
− | |||
− | |||
− | |||
− | |||
+ | * [https://github.com/dlang/tools/blob/master/setup.sh tools/setup.sh] is a simple script that either installs a new or updates an existing D development tree. Just download the script and run. | ||
<syntaxhighlight lang=bash> | <syntaxhighlight lang=bash> | ||
− | + | wget https://raw.githubusercontent.com/dlang/tools/master/setup.sh && bash setup.sh | |
+ | *** The following projects will be INSTALLED: | ||
+ | <YOUR-CURRENT-PATH>/dmd | ||
+ | <YOUR-CURRENT-PATH>/druntime | ||
+ | <YOUR-CURRENT-PATH>/phobos | ||
+ | <YOUR-CURRENT-PATH>/dlang.org | ||
+ | <YOUR-CURRENT-PATH>/tools | ||
+ | <YOUR-CURRENT-PATH>/installer | ||
+ | <YOUR-CURRENT-PATH>/dub | ||
+ | Is this what you want? [y|n] | ||
+ | y | ||
</syntaxhighlight> | </syntaxhighlight> | ||
− | + | * [https://github.com/CyberShadow/Digger Digger] can download and build D from any point in its recent history. | |
+ | * [https://github.com/jacob-carlborg/dvm DVM] can build and locally install D from source code. | ||
− | + | There are also the [https://dlang.org/download.html#dmd-nightly dmd-nightly builds]. | |
− | |||
− | |||
− | + | == Building from source == | |
− | + | The build information is split into Posix and Windows pages. Be sure to follow the instructions in the related guide and come back to this page once your setup is working. | |
− | + | * [[Building_under_Posix | Building under Posix (Linux, macOS, FreeBSD, ...)]] | |
− | + | * [[Building_under_Windows | Building under Windows]] | |
− | |||
− | |||
− | + | Once Druntime and Phobos are compiled, you can use your freshly compiled DMD compiler to compile programs: | |
<syntaxhighlight lang=bash> | <syntaxhighlight lang=bash> | ||
− | + | ~/dlang/dmd/generated/linux/release/64/dmd mytest.d | |
</syntaxhighlight> | </syntaxhighlight> | ||
− | + | On a different OS, you will need to replace the OS name. For example for <tt>macOS</tt>: | |
<syntaxhighlight lang=bash> | <syntaxhighlight lang=bash> | ||
− | + | ~/dlang/dmd/generated/osx/release/64/dmd mytest.d | |
</syntaxhighlight> | </syntaxhighlight> | ||
− | == | + | == Development == |
− | + | Depending on which part of the D ecosystem you want to dive in, there are separate guides which explain e.g. how unittests are run: | |
− | + | * [[DMD_development | DMD Development]], [https://github.com/dlang/dmd/blob/master/CONTRIBUTING.md DMD Contribution Guide], and [[DMD_Source_Guide | DMD Source Guide]] | |
+ | * [[DRuntime_development | DRuntime development]] | ||
+ | * [[Contributing_to_Phobos | Phobos development]] | ||
+ | * [[Contributing_to_dlang.org | Dlang.org development]] (D documentation) | ||
− | < | + | There is also <tt>[https://github.com/dlang/tools tools]</tt> (typically cloned to the <tt>~/dlang/tools</tt> folder) where small helper programs live. |
− | + | For a full description of the tools provided, please see <tt>[https://github.com/dlang/tools tools]</tt>. | |
− | + | If you are on Windows, {{code|dtab}} (transforms tabs into spaces in source code) and {{code|tolf}} (replaces line endings with LF) might prove helpful. | |
− | + | Additionally, there are other repositories which are parts of the D ecosystem: <tt>[https://github.com/dlang/dconf.org dconf.org]</tt>, <tt>[https://github.com/dlang/dub dub]</tt>, <tt>[https://github.com/dlang/dub-registry dub-registry]</tt>, <tt>[https://github.com/dlang/installer installer]</tt>, <tt>[https://github.com/dlang/tools tools]</tt>, <tt>[https://github.com/dlang/visuald visuald]</tt>, <tt>[https://github.com/dlang/ci ci]</tt>, <tt>[https://github.com/dlang/undeaD undeaD]</tt> and more. | |
− | + | These ancillary repositories are of somewhat specific interest. Their installation process mimics that of the repositories described above. If you get to the point where you need to work on one of these, chances are you're already versed in what needs doing. If not, [http://forum.dlang.org/group/digitalmars.D ask away]. | |
− | + | == Typical Contributor Workflow == | |
− | + | There are many ways to use <tt>git</tt> and GitHub to contribute. Here's a typical one. | |
− | + | First, fork the github repository or repositories you'd like to contribute to (<tt>dmd</tt>, <tt>druntime</tt>, <tt>phobos</tt> etc) by navigating to their respective pages on <tt>github.com</tt> and clicking "Fork" (e.g. [https://github.com/dlang/dmd dmd], [https://github.com/dlang/druntime druntime], and [https://github.com/dlang/phobos phobos]). Then, set up your local git repository to reflect that. For example, consider you want to contribute to <tt>phobos</tt> and have forked it. Then run these commands: | |
<syntaxhighlight lang=bash> | <syntaxhighlight lang=bash> | ||
− | cd ~/ | + | cd ~/dlang/phobos |
− | git | + | git remote set-url origin git@github.com:USERNAME/phobos.git |
− | |||
− | |||
</syntaxhighlight> | </syntaxhighlight> | ||
− | + | (Replace <tt>USERNAME</tt> with your actual github user name.) | |
− | < | + | This sets your forked repository as origin. |
+ | If you <b>don't have</b> the phobos repository, go back to the [[#Building from source|Building from source section]] or make a fresh clone from your fork: | ||
− | + | <syntaxhighlight lang=bash> | |
− | + | cd ~/dlang | |
− | + | git clone git@github.com:username/phobos.git | |
− | |||
− | |||
− | |||
− | |||
− | |||
− | |||
− | |||
− | |||
− | |||
− | |||
− | |||
− | |||
− | |||
− | |||
− | |||
− | |||
− | |||
− | |||
− | |||
− | |||
− | |||
− | |||
− | |||
− | |||
− | |||
− | |||
</syntaxhighlight> | </syntaxhighlight> | ||
− | + | Now within your Phobos repository, setup the official Phobos repository as upstream remote node: | |
− | |||
− | |||
− | |||
− | |||
− | |||
− | |||
− | |||
− | |||
− | |||
− | |||
− | |||
<syntaxhighlight lang=bash> | <syntaxhighlight lang=bash> | ||
− | cd ~/ | + | cd ~/dlang/phobos |
− | git remote add | + | git remote add upstream https://github.com/dlang/phobos.git |
− | git | + | git fetch && git fetch upstream |
</syntaxhighlight> | </syntaxhighlight> | ||
− | + | Then, it's best to work in branches as shown below: | |
<syntaxhighlight lang=bash> | <syntaxhighlight lang=bash> | ||
Line 143: | Line 96: | ||
# ... get some good work done here ... | # ... get some good work done here ... | ||
git commit -am "Awesome new feature ..." | git commit -am "Awesome new feature ..." | ||
− | git push | + | git push myfork |
</syntaxhighlight> | </syntaxhighlight> | ||
With this, your work is in your github fork of the <tt>phobos</tt> (or whichever) repository. After that, visit your fork on <tt>github.com</tt>, which looks like <tt>https://github.com/username/phobos/tree/awesome-new-feature</tt>. | With this, your work is in your github fork of the <tt>phobos</tt> (or whichever) repository. After that, visit your fork on <tt>github.com</tt>, which looks like <tt>https://github.com/username/phobos/tree/awesome-new-feature</tt>. | ||
+ | Also, you can always pull-in the latest, greatest Phobos changes: | ||
+ | |||
+ | <syntaxhighlight lang=bash> | ||
+ | git checkout master | ||
+ | git fetch && git fetch upstream && git merge --ff-only upstream/master | ||
+ | </syntaxhighlight> | ||
== Create a pull request == | == Create a pull request == | ||
Once you have tested all your changes and pushed them to your fork on GitHub, you are ready to submit a pull request. | Once you have tested all your changes and pushed them to your fork on GitHub, you are ready to submit a pull request. | ||
+ | Navigate to <tt>[https://github.com/dlang/phobos phobos]</tt> GitHub page and you should already see your branch: | ||
+ | |||
+ | [[File:Phobos_start_new_PR.png|800px]] | ||
+ | |||
+ | Alternatively you can also click on the "New pull request" button, select "Compare across forks", select your repository as "head fork" and select the branch that you made your changes in, say issue_1234. | ||
− | + | [[File:PR_compare_across_changes.png|800px]] | |
− | |||
− | |||
This will submit your changes for review by the D maintainers. If your changes are approved, they will be merged into the master branch. Otherwise, if the maintainers have some comments or feedback, you can refine your changes by editing and testing in your local workspace, and pushing the new changes to the same git branch. The new changes will be automatically included in your pull request. | This will submit your changes for review by the D maintainers. If your changes are approved, they will be merged into the master branch. Otherwise, if the maintainers have some comments or feedback, you can refine your changes by editing and testing in your local workspace, and pushing the new changes to the same git branch. The new changes will be automatically included in your pull request. | ||
− | Choose a title for your pull request that clearly states what it does. When fixing a bug, the usual thing to do is to use the | + | === How to choose a title for your Pull Request === |
+ | |||
+ | Choose a title for your pull request that clearly states what it does. When fixing a bug, the usual thing to do is to use the title from the bugzilla report. Eg a title like "Fix 3797" or "Issue 3797" contains much less information than "Fix Issue 3797 - Regression(2.038): Implicit conversion between incompatible function pointers" and requires a lot more effort for the reviewers to determine if it is something they are interested in. | ||
+ | |||
+ | If a pull request isn't a trivial bug, its description should explain the motivation for the change and briefly summarize the changes. | ||
− | + | === Submitting a bug fix === | |
− | + | If the pull request is for a bug fix, the commit message should have the format "fix issue 1234". This allow the [https://github.com/dlang-bots/dlang-bot Dlang-Bot] to automatically pick up the issue from [[Bugzilla]] and post it as a comment on the PR. | |
+ | It's important the bot can link your commit message to the respective Bugzilla issue as it will then automatically close the issue on Bugzilla and otherwise the fixed issue won't appear in the changelog. | ||
− | === Characteristics of a | + | If the PR is already open, then a <tt>git rebase</tt> is necessary followed by a force push. During the rebase, the commit message should be renamed to match the "Fix <copy bugzilla title>" |
+ | |||
+ | [[File:Bugzilla_issue_title.png]] | ||
+ | |||
+ | For [https://issues.dlang.org/show_bug.cgi?id=9582 Issue 9582], the commit message could be <tt>Fix Issue 9582 - std.algorithm.map!(T) cause CT error for fixed size arrays</tt>. | ||
+ | |||
+ | After the pull request is created, don't forget to add a link to your pull request posted to the corresponding Bugzilla issue in a comment, so that future contributor are aware of your work and don't redo the fix. | ||
+ | Hence, it's also helpful to add the 'pull' keyword to the Bugzilla issue. | ||
+ | |||
+ | === Characteristics of a good pull request === | ||
* Addresses one topic only. | * Addresses one topic only. | ||
Line 177: | Line 153: | ||
* If the PR introduces a regression, a large diff is much harder to debug than a small one. | * If the PR introduces a regression, a large diff is much harder to debug than a small one. | ||
* They imply an all or nothing view of it, when it actually may have good parts and bad parts. | * They imply an all or nothing view of it, when it actually may have good parts and bad parts. | ||
+ | |||
+ | == Working on an pull request == | ||
=== Autotester === | === Autotester === | ||
Line 186: | Line 164: | ||
=== Rebasing === | === Rebasing === | ||
− | Sometimes, if a particular change you are working on is taking a long time, or if you encounter a problem that is fixed by a new commit upstream, you may need to sync your local branch with master in order to keep the code up-to-date. In this case, it is recommended that you use git rebase to apply your changes ''on top of'' the latest git master, so that when you submit a pull request, the change history will be easier for the reviewers to follow. | + | Sometimes, if a particular change you are working on is taking a long time, or if you encounter a problem that is fixed by a new commit upstream, you may need to sync your local branch with master in order to keep the code up-to-date. In this case, it is recommended that you use git rebase to apply your changes ''on top of'' the latest git master, so that when you submit a pull request, the change history will be easier for the reviewers to follow. git merge should <b>not</b> be used, as it may produce a lot of merge commits that may not be relevant to your changes. |
For example, you may be working on your changes: | For example, you may be working on your changes: | ||
<syntaxhighlight lang=bash> | <syntaxhighlight lang=bash> | ||
− | cd / | + | cd ~/dlang/phobos |
git checkout mybranch | git checkout mybranch | ||
vim std/algorithm.d # apply lots of cool changes here | vim std/algorithm.d # apply lots of cool changes here | ||
Line 199: | Line 177: | ||
<syntaxhighlight lang=bash> | <syntaxhighlight lang=bash> | ||
− | git commit - | + | git commit --ammend |
</syntaxhighlight> | </syntaxhighlight> | ||
Line 215: | Line 193: | ||
</syntaxhighlight> | </syntaxhighlight> | ||
− | The --ff-only option is to ensure that your master branch is identical to the official D sources' master branch, since otherwise you will end up with a very messy history that will be hard to clean up (and the reviewers will probably reject your pull request due to having unrelated merge commits). | + | The <tt>--ff-only</tt> option is to ensure that your master branch is identical to the official D sources' master branch, since otherwise you will end up with a very messy history that will be hard to clean up (and the reviewers will probably reject your pull request due to having unrelated merge commits). |
Now go back to your branch and rebase it: | Now go back to your branch and rebase it: | ||
Line 226: | Line 204: | ||
Now your sources should be up-to-date. Recompile and test everything to make sure it all works. | Now your sources should be up-to-date. Recompile and test everything to make sure it all works. | ||
− | Note that after rebasing, you will need to force an update to your fork on GitHub with the - | + | Note that after rebasing, you will need to force an update to your fork on GitHub with the --force-with-lease flag, otherwise it will fail because the histories don't match anymore: |
<syntaxhighlight lang=bash> | <syntaxhighlight lang=bash> | ||
− | git push - | + | git push --force-with-lease origin mybranch |
</syntaxhighlight> | </syntaxhighlight> | ||
Line 254: | Line 232: | ||
<syntaxhighlight lang=bash> | <syntaxhighlight lang=bash> | ||
git commit --amend | git commit --amend | ||
− | git push - | + | git push --force-with-lease |
</syntaxhighlight> | </syntaxhighlight> | ||
Line 272: | Line 250: | ||
Then follow the instructions for [[#Make your changes in a branch|making a branch]]. | Then follow the instructions for [[#Make your changes in a branch|making a branch]]. | ||
− | + | Unfortunately, it's not possible to simply re-target your branch for pulling into the stable branch. GitHub will let you do this, but your branch will include many of the changes from the unstable branch! | |
In order to fix such a problem, you can [[#Rebasing|rebase]] your changes from master on top of the stable branch. First you need to pull in the stable branch from your fork on github: | In order to fix such a problem, you can [[#Rebasing|rebase]] your changes from master on top of the stable branch. First you need to pull in the stable branch from your fork on github: | ||
Line 290: | Line 268: | ||
You may have to follow the instructions in the [[#Rebasing|Rebasing section]] on adding the upstream branch, substituting stable for master, if you need to update to the latest stable changes. | You may have to follow the instructions in the [[#Rebasing|Rebasing section]] on adding the upstream branch, substituting stable for master, if you need to update to the latest stable changes. | ||
− | This sometimes may not work, as the changes between the stable and master are too drastic. In this case, you may have to re create your changes after a clean checkout of the stable branch. | + | This sometimes may not work, as the changes between the stable and master are too drastic. In this case, you may have to re-create your changes after a clean checkout of the stable branch. |
When creating a pull request, you need to tell github to target the stable branch instead of master on the upstream repository. This is done via a drop-down at the top of the page, make sure to do this before submitting your pull request as this cannot be changed after the PR is created (you will have to close the PR and create a new one). | When creating a pull request, you need to tell github to target the stable branch instead of master on the upstream repository. This is done via a drop-down at the top of the page, make sure to do this before submitting your pull request as this cannot be changed after the PR is created (you will have to close the PR and create a new one). | ||
Line 311: | Line 289: | ||
What you need to do is rebase your git branch on the master branch. What this does is rewrite the history of your git branch to make it seem like it was merged off of the head of master rather than the older commit where you actually branched. This will include the new commits in your PR so your PR no longer conflicts. See [https://github.com/edx/edx-platform/wiki/How-to-Rebase-a-Pull-Request this tutorial] for more details. | What you need to do is rebase your git branch on the master branch. What this does is rewrite the history of your git branch to make it seem like it was merged off of the head of master rather than the older commit where you actually branched. This will include the new commits in your PR so your PR no longer conflicts. See [https://github.com/edx/edx-platform/wiki/How-to-Rebase-a-Pull-Request this tutorial] for more details. | ||
+ | |||
+ | === I would like to help but I don't know how. What is the best way? === | ||
+ | |||
+ | Anyone is welcome to contribute as D is very much a volunteer effort! | ||
+ | The best place to look for ideas to contribute is the [[Get involved|get involved]] guide. | ||
+ | Please also don't hesitate to point out (or even fix) any stumbling blocks you may encounter when starting out. | ||
+ | |||
+ | === CodeCov shows a red cross === | ||
+ | |||
+ | [[File:CI_CodeCov_failed.png]] | ||
+ | |||
+ | On every build the generated coverage files are sent to CodeCov for further processing. | ||
+ | You can see the non-covered lines by clicking on the link or for further convenience in the future by installing the [https://github.com/codecov/browser-extension CodeCov browser extension]. | ||
+ | |||
+ | === CircleCi shows a red cross === | ||
+ | |||
+ | [[File:CI_CircleCI_failed.png]] | ||
+ | |||
+ | On CircleCi static code analysis is run. For example it lints the Phobos codebase after [https://dlang.org/dstyle.html the DStyle]. | ||
+ | Moreover, checks are run to ensure that every example on dlang.org is runnable. On Posix, you can run this locally by executing the <tt>style</tt> target: | ||
+ | |||
+ | <syntaxhighlight lang=bash> | ||
+ | make -f posix.mak style | ||
+ | </syntaxhighlight> | ||
+ | |||
+ | === The auto-tester shows a red cross === | ||
+ | |||
+ | [[File:CI_auto-tester_failed.png]] | ||
+ | |||
+ | The [https://auto-tester.puremagic.com auto-tester] tests every PR on all supported platforms (Windows, Linux, macOS, and Linux) for 32 and 64-bit builds. | ||
+ | Please click on the link of your PR to find out more about the error. | ||
+ | |||
+ | === The Jenkins Project Tester shows a red cross === | ||
+ | |||
+ | [[File:CI_project_tester.png]] | ||
+ | |||
+ | The Project Tester tests every PR on all supported platforms (Windows, Linux, macOS, and Linux) for 32 and 64-bit builds. | ||
+ | Please click on the link of your PR to find out more about the error. | ||
+ | |||
+ | === How can I link a PR with Bugzilla? === | ||
+ | |||
+ | [[File:DLang_Bot_comment.png]] | ||
+ | |||
+ | The integration is handled by [https://github.com/dlang-bots/dlang-bot Dlang-Bot]. | ||
+ | If you mention a bugzilla issue in your git commit message, you will get linked to the Bugzilla issue. | ||
+ | For more information, please visit [https://github.com/dlang-bots/dlang-bot Dlang-Bot's documentation]. | ||
== See Also == | == See Also == | ||
+ | * [https://github.com/dlang/dmd/blob/master/CONTRIBUTING.md Contributing to DMD] | ||
+ | * [[DMD Source Guide]] | ||
* [[Contributing_to_Phobos| Contributing to Phobos]] | * [[Contributing_to_Phobos| Contributing to Phobos]] | ||
* [[Contributing_to_dlang.org| Contributing to dlang.org]] | * [[Contributing_to_dlang.org| Contributing to dlang.org]] | ||
+ | * [[Guidelines for maintainers]] | ||
* [[Get involved]] | * [[Get involved]] | ||
− | |||
[[Category: Contribution Guidelines]] | [[Category: Contribution Guidelines]] |
Latest revision as of 18:28, 11 December 2019
This page describes how to build D, put together a correct patch, and contribute it as a GitHub pull request.
Contents
- 1 Existing tools
- 2 Building from source
- 3 Development
- 4 Typical Contributor Workflow
- 5 Create a pull request
- 6 Working on an pull request
- 7 Contributing FAQ
- 7.1 A file that I made a change on was modified by a different merged PR, and now my PR can't be merged, now what?
- 7.2 I would like to help but I don't know how. What is the best way?
- 7.3 CodeCov shows a red cross
- 7.4 CircleCi shows a red cross
- 7.5 The auto-tester shows a red cross
- 7.6 The Jenkins Project Tester shows a red cross
- 7.7 How can I link a PR with Bugzilla?
- 8 See Also
Existing tools
If you only want to build the latest and greatest DMD, there exist tools which can do this automatically:
- tools/setup.sh is a simple script that either installs a new or updates an existing D development tree. Just download the script and run.
wget https://raw.githubusercontent.com/dlang/tools/master/setup.sh && bash setup.sh
*** The following projects will be INSTALLED:
<YOUR-CURRENT-PATH>/dmd
<YOUR-CURRENT-PATH>/druntime
<YOUR-CURRENT-PATH>/phobos
<YOUR-CURRENT-PATH>/dlang.org
<YOUR-CURRENT-PATH>/tools
<YOUR-CURRENT-PATH>/installer
<YOUR-CURRENT-PATH>/dub
Is this what you want? [y|n]
y
- Digger can download and build D from any point in its recent history.
- DVM can build and locally install D from source code.
There are also the dmd-nightly builds.
Building from source
The build information is split into Posix and Windows pages. Be sure to follow the instructions in the related guide and come back to this page once your setup is working.
Once Druntime and Phobos are compiled, you can use your freshly compiled DMD compiler to compile programs:
~/dlang/dmd/generated/linux/release/64/dmd mytest.d
On a different OS, you will need to replace the OS name. For example for macOS:
~/dlang/dmd/generated/osx/release/64/dmd mytest.d
Development
Depending on which part of the D ecosystem you want to dive in, there are separate guides which explain e.g. how unittests are run:
- DMD Development, DMD Contribution Guide, and DMD Source Guide
- DRuntime development
- Phobos development
- Dlang.org development (D documentation)
There is also tools (typically cloned to the ~/dlang/tools folder) where small helper programs live. For a full description of the tools provided, please see tools. If you are on Windows, dtab (transforms tabs into spaces in source code) and tolf (replaces line endings with LF) might prove helpful.
Additionally, there are other repositories which are parts of the D ecosystem: dconf.org, dub, dub-registry, installer, tools, visuald, ci, undeaD and more.
These ancillary repositories are of somewhat specific interest. Their installation process mimics that of the repositories described above. If you get to the point where you need to work on one of these, chances are you're already versed in what needs doing. If not, ask away.
Typical Contributor Workflow
There are many ways to use git and GitHub to contribute. Here's a typical one.
First, fork the github repository or repositories you'd like to contribute to (dmd, druntime, phobos etc) by navigating to their respective pages on github.com and clicking "Fork" (e.g. dmd, druntime, and phobos). Then, set up your local git repository to reflect that. For example, consider you want to contribute to phobos and have forked it. Then run these commands:
cd ~/dlang/phobos
git remote set-url origin git@github.com:USERNAME/phobos.git
(Replace USERNAME with your actual github user name.)
This sets your forked repository as origin. If you don't have the phobos repository, go back to the Building from source section or make a fresh clone from your fork:
cd ~/dlang
git clone git@github.com:username/phobos.git
Now within your Phobos repository, setup the official Phobos repository as upstream remote node:
cd ~/dlang/phobos
git remote add upstream https://github.com/dlang/phobos.git
git fetch && git fetch upstream
Then, it's best to work in branches as shown below:
git checkout -b awesome-new-feature
# ... get some good work done here ...
git commit -am "Awesome new feature ..."
git push myfork
With this, your work is in your github fork of the phobos (or whichever) repository. After that, visit your fork on github.com, which looks like https://github.com/username/phobos/tree/awesome-new-feature. Also, you can always pull-in the latest, greatest Phobos changes:
git checkout master
git fetch && git fetch upstream && git merge --ff-only upstream/master
Create a pull request
Once you have tested all your changes and pushed them to your fork on GitHub, you are ready to submit a pull request. Navigate to phobos GitHub page and you should already see your branch:
Alternatively you can also click on the "New pull request" button, select "Compare across forks", select your repository as "head fork" and select the branch that you made your changes in, say issue_1234.
This will submit your changes for review by the D maintainers. If your changes are approved, they will be merged into the master branch. Otherwise, if the maintainers have some comments or feedback, you can refine your changes by editing and testing in your local workspace, and pushing the new changes to the same git branch. The new changes will be automatically included in your pull request.
How to choose a title for your Pull Request
Choose a title for your pull request that clearly states what it does. When fixing a bug, the usual thing to do is to use the title from the bugzilla report. Eg a title like "Fix 3797" or "Issue 3797" contains much less information than "Fix Issue 3797 - Regression(2.038): Implicit conversion between incompatible function pointers" and requires a lot more effort for the reviewers to determine if it is something they are interested in.
If a pull request isn't a trivial bug, its description should explain the motivation for the change and briefly summarize the changes.
Submitting a bug fix
If the pull request is for a bug fix, the commit message should have the format "fix issue 1234". This allow the Dlang-Bot to automatically pick up the issue from Bugzilla and post it as a comment on the PR. It's important the bot can link your commit message to the respective Bugzilla issue as it will then automatically close the issue on Bugzilla and otherwise the fixed issue won't appear in the changelog.
If the PR is already open, then a git rebase is necessary followed by a force push. During the rebase, the commit message should be renamed to match the "Fix <copy bugzilla title>"
For Issue 9582, the commit message could be Fix Issue 9582 - std.algorithm.map!(T) cause CT error for fixed size arrays.
After the pull request is created, don't forget to add a link to your pull request posted to the corresponding Bugzilla issue in a comment, so that future contributor are aware of your work and don't redo the fix. Hence, it's also helpful to add the 'pull' keyword to the Bugzilla issue.
Characteristics of a good pull request
- Addresses one topic only.
- Refactorings should not be mixed in with bug fixes or enhancements.
- Refactorings should be marked as such in the subject, and must not contain any behavior changes.
- Larger refactorings need to broken up into smaller PRs.
The problems with large PRs are:
- They are hard to review.
- github is very slow at serving pages with large diffs on them.
- If the PR introduces a regression, a large diff is much harder to debug than a small one.
- They imply an all or nothing view of it, when it actually may have good parts and bad parts.
Working on an pull request
Autotester
Pull requests are automatically picked up by the autotester, which compiles the code in the pull request and runs it through the dmd, druntime, and phobos unittests on all supported platforms. Generally, pull requests must pass all tests before they will be merged. The status of the tests can be monitored through the pull request page.
Every user must be manually approved before the autotester will start testing their pull requests. Users can be approved by anyone with commit access.
Rebasing
Sometimes, if a particular change you are working on is taking a long time, or if you encounter a problem that is fixed by a new commit upstream, you may need to sync your local branch with master in order to keep the code up-to-date. In this case, it is recommended that you use git rebase to apply your changes on top of the latest git master, so that when you submit a pull request, the change history will be easier for the reviewers to follow. git merge should not be used, as it may produce a lot of merge commits that may not be relevant to your changes.
For example, you may be working on your changes:
cd ~/dlang/phobos
git checkout mybranch
vim std/algorithm.d # apply lots of cool changes here
First, before you resync with master, make sure all your changes are checked in (or stashed):
git commit --ammend
If you forked from the official D programming language repositories you may need to add an upstream remote to pull in the latest official changes. If this is the case you can add an upstream remote like this:
git remote add upstream git@github.com:dlang/phobos
This adds another remote to your repository called upstream and only needs to be done once. Once the upstream remote is added, you can update your repository's master branch by running the following:
git checkout master
git pull --ff-only upstream master
The --ff-only option is to ensure that your master branch is identical to the official D sources' master branch, since otherwise you will end up with a very messy history that will be hard to clean up (and the reviewers will probably reject your pull request due to having unrelated merge commits).
Now go back to your branch and rebase it:
git checkout mybranch
git rebase master
Now your sources should be up-to-date. Recompile and test everything to make sure it all works.
Note that after rebasing, you will need to force an update to your fork on GitHub with the --force-with-lease flag, otherwise it will fail because the histories don't match anymore:
git push --force-with-lease origin mybranch
You may wish to read up on how git rebase works if you're not familiar with the concept.
If, during the 'git rebase' command, you encounter conflicts, you may want to learn how to resolve a conflict during git rebase.
Squashing
After receiving feedback on your PR, it's common for it to have lots of commits that don't add much by being separate. For example, consider the following git history on a PR:
commit [ffffff] Added new function: foobar commit [aaaaaa] Spelling error fix in foobar docs commit [cccccc] Clarified Docs for foobar
Nothing is gained from having these as three separate commits as they are all focused on one feature. Instead, they should be one commit so the history looks like this
commit [333333] Added new function: foobar
while still retaining all of your changes. In order to perform this, please consult this guide
You can also directly append to your last commit and force an update of your PR:
git commit --amend
git push --force-with-lease
Stable Branch
If you are working on a fix for a regression, chances are it should go into the next point release, and not the next major version (e.g. 2.067.1 instead of 2.068). In this case, you should check out the stable branch of each subproject BEFORE you create your topic branch:
cd dmd
git checkout stable
cd ../druntime
git checkout stable
cd ../phobos
git checkout stable
Then follow the instructions for making a branch.
Unfortunately, it's not possible to simply re-target your branch for pulling into the stable branch. GitHub will let you do this, but your branch will include many of the changes from the unstable branch!
In order to fix such a problem, you can rebase your changes from master on top of the stable branch. First you need to pull in the stable branch from your fork on github:
git checkout stable
Then, you go back to your branch, and replay the changes from master using rebase:
git checkout mybranch
git fetch upstream
git rebase --onto upstream/stable upstream/master mybranch
You may have to follow the instructions in the Rebasing section on adding the upstream branch, substituting stable for master, if you need to update to the latest stable changes.
This sometimes may not work, as the changes between the stable and master are too drastic. In this case, you may have to re-create your changes after a clean checkout of the stable branch.
When creating a pull request, you need to tell github to target the stable branch instead of master on the upstream repository. This is done via a drop-down at the top of the page, make sure to do this before submitting your pull request as this cannot be changed after the PR is created (you will have to close the PR and create a new one).
If you notice in your PR a whole slew of changes that seem to have nothing to do with your changes, it's likely because you forgot one of these steps.
Reviews
Any pull requests that make language changes must be approved by Walter and Andrei. This includes druntime changes that implement the specification.
Any pull requests that make significant changes to code should be reviewed by more than one person. This means that at least two people need to approve the pull request before it is merged. One person must be a person with commit rights, but the other need not be, as long as that person is trusted within the developer community.
Pull requests that are trivial (typos, obvious minor bug fixes, etc.) may be pulled without a second review.
Please note that any updates pushed to the candidate branch do not automatically notify a subscribed person. If you update your branch to correct an issue, please also put in a comment indicating it.
Contributing FAQ
A file that I made a change on was modified by a different merged PR, and now my PR can't be merged, now what?
What you need to do is rebase your git branch on the master branch. What this does is rewrite the history of your git branch to make it seem like it was merged off of the head of master rather than the older commit where you actually branched. This will include the new commits in your PR so your PR no longer conflicts. See this tutorial for more details.
I would like to help but I don't know how. What is the best way?
Anyone is welcome to contribute as D is very much a volunteer effort! The best place to look for ideas to contribute is the get involved guide. Please also don't hesitate to point out (or even fix) any stumbling blocks you may encounter when starting out.
CodeCov shows a red cross
On every build the generated coverage files are sent to CodeCov for further processing. You can see the non-covered lines by clicking on the link or for further convenience in the future by installing the CodeCov browser extension.
CircleCi shows a red cross
On CircleCi static code analysis is run. For example it lints the Phobos codebase after the DStyle. Moreover, checks are run to ensure that every example on dlang.org is runnable. On Posix, you can run this locally by executing the style target:
make -f posix.mak style
The auto-tester shows a red cross
The auto-tester tests every PR on all supported platforms (Windows, Linux, macOS, and Linux) for 32 and 64-bit builds. Please click on the link of your PR to find out more about the error.
The Jenkins Project Tester shows a red cross
The Project Tester tests every PR on all supported platforms (Windows, Linux, macOS, and Linux) for 32 and 64-bit builds. Please click on the link of your PR to find out more about the error.
How can I link a PR with Bugzilla?
The integration is handled by Dlang-Bot. If you mention a bugzilla issue in your git commit message, you will get linked to the Bugzilla issue. For more information, please visit Dlang-Bot's documentation.