Contributing to Phobos

Welcome to the D community and thanks for your interest in contributing! If you need help you can ask questions on #d IRC channel on freenode.org (web interface) or on our forum.

For a lists of ideas on how to contribute, please see Get involved. This document provides advice on making a good pull request to Phobos.

Guidelines for contributing to Phobos

The usual workflow is:

1. Fork Phobos on Github
2. Create your own branch
3. Work locally on your new feature or fix (see the tips below)
4. Test your improvements locally
5. Submit your pull request (PR)

You can find build instructions here. Below is a list of tips and checks that you should consider before opening your pull request:

Unittest phobos

This section explains commonly commands which are commonly used to test the D standard library phobos.

Full build

If you want to work on phobos itself, you need to run unittests—either for the full library, a package, or a module. To unittest the entire library:

make -j16 -f posix.mak unittest

Adjust the parameter passed to -j depending on your machine (beefier machines support larger parameters). This command unittests phobos in both debug and release mode. To only test one of them, add BUILD=debug or BUILD=release to the command line, for example:

make -j16 -f posix.mak BUILD=debug unittest

Specifying BUILD makes unittesting faster so it is recommended in iterative development. Just make sure both debug and release builds are tested before e.g. submitting a pull request.

Test a single Phobos module

While changing one specific package or module, it's useful to be able to only unittest that particular entity. The following two commands only unittest (in debug mode) the std.algorithm package:

make -j16 -f posix.mak BUILD=debug std/algorithm.test

Several modules, packages, or mix thereof may be specified for testing in the same command line. For example, this command also tests (and also in debug mode) the std.algorithm package and the std.conv module, with better parallelism:

make -j16 -f posix.mak BUILD=debug std/algorithm.test std/conv.test

Test a single module without rebuilding Phobos

You can test a module directly with rdmd:

rdmd --compiler=~/dlang/dmd/generated/linux/release/64/dmd -unittest -main std/algorithm/sorting.d

(If you are on a different platform, replace linux> with your respective platform)

However, be aware that this method doesn't recompile the Phobos library and thus only checks the executed file. Thus you should only use this for fast builds on small changes.

Run CI checks

The auto-tester will fail your PR if your changes contain trailing whitespace or incorrect line endings. You can run this test locally with the command:

make -j16 -f posix.mak checkwhitespace

CircleCi will also run various checks for the DStyle and ensures that every example is runnable on dlang.org. You can run this test locally with:

make -f posix.mak style

Code

Code style

• Stick to the style guide? (e.g. braces in their own lines (allman style) or white spaces between operators)
• Avoid unnecessarily importing other Phobos modules
• Try to not require the garbage collector (this is not a must)
• Avoid code duplication
• Avoid using auto as return type wherever possible (makes the documentation easier to read)
• Use static nested structs for ranges (aka Voldemort types) and thus store function parameters explicitly in the nested struct
• Avoid unittest in templates (it will generate a new unittest for each instance) - put your tests outside
• Do overflow checks when computing sizes for malloc and realloc

Code advice

• Don't be afraid to use the new compiler or language features (e.g. Foo!Range vs. Foo!(Range))
• Be critical when looking at Phobos code (some parts of Phobos are a bit older and have some relics due to lacking functionality of the compiler)
• Is there an easier or more performant way to do it?
• Does your PR address only one topic? Can it be split up in smaller PRs?
• Is your code flexible enough to accommodate more use cases?
• Read through your code again - is there any part that is not understandable?

Tests

• Autotester will automatically compile the code in your PR and test it on all supported platforms. For your first PR you need approval from any reviewer - ping them in case they forget.
• Do all tests pass locally? You can run the tests of
  • single module or packages with make -f posix.mak std/algorithm/comparison.test or make -f posix.mak std/algorithm.test
  • all tests with make -f posix.mak unittest (add -j NUMBER_OF_CORES if you have multiple cores)
  • for small changes in a single module faster with rdmd -main -unittest comparison.d (be careful, this links to your current Phobos version)
  • for Windows have a look at building Phobos under windows)
• Do your tests cover all cases? (you can check code coverage in the resulting module.lst of rdmd -cov -main -unittest module.d)

Review process

• All PRs must pass all tests for all supported platforms of autotester before they will be merged
• Big additions (new modules) should go to std.experimental first (after they pass the review process)
• Smaller additions like individual functions can be merged directly after @andralex approves
• Refactoring or bug fixes just need approval (LGTM) from two reviewers and enough time passed (usually 2-3 days, depending on weekends and holidays) to give everyone an opportunity to shout
• For low-level changes the other compiler devs (GDC and LLVM) should be consulted
• Trivial changes (e.g. small documentation enhancements) are usually merged in less than 24h - otherwise ping for reviewers
• If your PR is stalled (no activity for 2-3 days) and you addressed all concerns, don't hesitate to ping your reviewers
• See here for whom to ping
• See rebasing if you need to sync your fork with master

Naming guidelines

General advices

• Phobos uses a distinct naming scheme
• Use a Capitalized short (50 chars or less) Git commit summary
• If necessary, wrap your Git explanation paragraph to 72 chars
• Do you use clear variable names?
• Does your PR have a concise and informative title?
• Are your Git messages clear and informative (otherwise use git commit --amend for a single commit or git rebase for multiple commits)?
• Ideally, every commit should contain one easily-explained, self-contained change. Squash fix-ups to your previous commits into the commit they target.

Issues

• Bug fix PRs should have a separate commit for each bugzilla issue that it fixes
• The message of the commit actually fixes the bug should start with "Fix issue X" where X is the number of the corresponding bugzilla issue. For example:

+ Refactor the code assist with the bug fix
+ Fix issue 17000 - map doesn't accept some range types
+ Fix issue 17001 - map doesn't work with some other range type

• Unrelated fixes, refactorings or additions should preferably go separate pull request(s) (tightly related fixes are okay in the same PR)

See the Dlang-Bot explanation on referencing issues for more details.

Documentation style

• The documentation is written in Ddoc
• Use complete English sentences with correct syntax, grammar, and punctuation
• Use precise technical terms. Avoids colloquialisms
• Honestly describe limitations and dangers
• Add Params, Returns and if needed See_Also sections
• Provide at least one ddoced unittest for a new method
• Have a brief one sentence summary for at the beginning of a method
• Give a synopsis at the beginning of a module accompanied by an example

Documentation checks

• Read your documentation text aloud - is it understandable?
• Check the output of CyberShadows's DAutoTest of the documentation (or run it yourself)
• All user impacting changes should have a corresponding changelog entry
• Did you add your method to the summary booktable or cheatsheat of a module? (might be in package.d)

Documentation tricks

• Use backticks `code` instead of $(D foo)
• Start with a Capital letter in all blocks of the method header
• Use $(REF) for links to other methods (format is $(REF _stripLeft, std, algorithm, mutation))
• Use $(REF_ALTTEXT) for a different link name (format is $(REF_ALTTEXT your text, formattedRead, std, format)
• Variables will be automatically put in backticks, use an underscore to avoid this behavior (e.g. _foo)
• Section headings are marked by a line with a single word followed by a colon :, use an underscore to avoid this behavior (e.g. with the following \n code: needs _code to avoid the creation of a section)
• Section headings with multiple words should use underscores instead of spaces (e.g. See_Also:)
• Don't put examples in your ddoc header - use the /// magic to annotate them. Ddoc will automatically add the annotated test(s) to the Examples section of the method, e.g.:

///
unittest
{
    assert(1 == 1);
}

• Use /// ditto to join multiple methods in the documentation

Happy hacking!

If you find a new gotcha, don't hesitate to edit this guide.

Thank you for your contribution!

See Also

• Starting as a Contributor
• Guidelines for maintainers
• Get involved