Contributing to Phobos

Guidelines for 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.

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)

Below is a list of tips and checks that you should consider before opening your pull request:

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 a garbage collector (this is not a must)
• Avoid code duplication
• Maximal @nogc, @safe, pure, nothrow in non-templated functions
• Don't add @nogc, @safe, pure, nothrow attributes to templated functions - let the compiler infer it! However add unittest checks for these attributes if possible (e.g. pure nothrow @nogc @safe unittest { ... })
• 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

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 72h - 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
• Ping @9il for anything math related
• See rebasing if you need to sync your fork with master

Naming

• 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)?
• Avoid having many commits per PR (usually one commit per PR) - see squashing to combine commits

Fixing 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)

Documentation

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!