Contributing to Phobos

Guidelines for Contributing

Welcome to the D community and thanks for your interest in contributing!

Quick links

• Fork on Github
• Use our Bugzilla bug tracker
• Follow the D style
• Participate in our forum
• Ask questions on #d IRC channel on freenode.org (web interface)
• Review Phobos additions in the Review Queue

How to make a good submission?

The usual workflow is:

1. Fork Phobos on Github
2. Create your own branch
3. [Starting as a Contributor#Building_D 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!