View source for Guidelines for maintainers

This document is intended to be a guide maintainer of <code>dlang</code> repositories on GitHub. However, it might contain interesting details for contributors who are interested in knowing more about the inner workings of the D GitHub processes.

== Quick checklist ==

* Do all CI pass?
* Are there any changes on the documentation or coverage output?
* Is it a regression fix? (check: it should be based ontro <code>stable</code>)
* Is it a non-trivial change? (check: either linkage with Bugzilla or a changelog entry)
* Is it a new symbols? (check: has it been pre-approved by Andrei?)

This is an initial list. Please help to extend it based on your own experiences.

== Review workflow (squashed commits & write access to PRs) ==

The ideal workflow is that a PR gets commits '''appended''' until its final approval, s.t. you only need to review the added changes.

=== General ideas ===

* PRs should only contain a small set of changes
* Contributors can prefix their appended commit messages with e.g. "[SQUASH]" (Please not, that this will squash all commits of a PR not only the ones marked. Additionally it is not a squash, but a fixup, which means, that commit messages get lost.)

GitHub has two features to help us here:

=== Commit squashing ===

* All commits get squashed into one commit before the merge
* This is enabled for all DLang repos
* "auto-merge-squash" does squashing as auto-merge behavior

For more infos please see the [https://github.com/blog/2141-squash-your-commits official article].

=== Write access to PRs ===

* This is an '''awesome''' feature that hasn't been used much so far
* It allows maintainer to do those nitpicks themselves (squashing all commits, fixing typos, ...) instead of going with the usual ping-pong cycle
* It's enabled by default for new PRs
* If someone turned it accidentally off, it's really okay to ask him/her as this is a massive time saver

<code>git</code> allows to define alliases in your <code>~/.gitconfig</code>:

<pre>
[alias]
pr  = "!f() { git fetch -fu ${2:-upstream} refs/pull/$1/head:pr/$1 && git checkout pr/$1; }; f"
</pre>

With this <code>git alias</code> you can checkout any PR:

<pre>
> git pr 5150
</pre>

In case you don't want to enter the branch to push to, you can use this small snippet:

<syntaxhighlight lang='bash'>
#!/bin/bash
tmpfile=$(mktemp)
repoSlug=$(git remote -v | grep '^upstream' | head -n1 | perl -lne 's/github.com:?\/?(.*)\/([^.]*)([.]git| )// or next; print $1,"/",$2')
prNumber=$(git rev-parse --abbrev-ref HEAD | cut -d/ -f 2)
curl -s https://api.github.com/repos/${repoSlug}/pulls/${prNumber} > $tmpfile
trap "{ rm -f $tmpfile; }" EXIT
headRef=$(cat $tmpfile | jq -r '.head.ref')
headSlug=$(cat $tmpfile | jq -r '.head.repo.full_name')
git push -f git@github.com:${headSlug} HEAD:${headRef}
</syntaxhighlight>

For more details on pushing changes to a PR, please see [https://help.github.com/articles/allowing-changes-to-a-pull-request-branch-created-from-a-fork the official article].

== Auto-Merge ==

There are "auto-merge" and "auto-merge-squash" labels which are based on an auto-merge system that takes the status of all required CIs into account (the auto-tester tries the merge after its test passed, but doesn't look out for other CIs.)
You can toggle a PR for auto-merge by simply adding this label or for the keyboard-enthusiasts: press "l", press "a" and hit enter.

Warning: this new auto-merge system is officially still WIP because:
- "auto-merge"-labelled PRs [https://github.com/dlang-bots/dlang-bot/pull/50 aren't yet prioritized on the auto-tester]
- We would like to set _all_ CIs to enforced

For more details, please see [https://github.com/dlang-bots/dlang-bot#auto-merge-wip its documentation].

== CI ==

* We are working on making the CIs more reliable. If you see a transient error, please let us know!
* In any other case, the "red cross" on a CI has a meaning - surprise!

Here's a small summary of the CIs and their tasks:

=== auto-tester ===

* runs autotester that automatically compiles and runs the dmd, druntime, and phobos unittests on all officially-supported platforms
* if there are spordiadic failures, they can be deprecated by clicking on the "Deprecate" button of a PR
* new contributors need to be approved (this can be done on the [https://auto-tester.puremagic.com main page] of the auto-tester)
* Maintainer: [https://github.com/braddr @braddr]
* [https://github.com/braddr/d-tester/issues Support repo]

=== DAutoTest ===

It builds the entire documentation and allows to preview changes.
In particular
* <code>/library-prerelease</code> is the DDox built on the documentation (one file per function)
* <code>/phobos-prerelease</code> is the DDoc built of the documentation (one file per module)
* for PRs to <code>master</code>: only <code>-prerelease</code> should appear in the list of changes (the documentation on <code>/library</code> or <code>/phobos</code> is built from the <code>stable</code> branch)
* Maintainer: [https://github.com/CyberShadow @CyberShadow]


Tip: install [https://chrome.google.com/webstore/detail/git-patch-viewer/hkoggakcdopbgnaeeidcmopfekipkleg Git Patch Viewer] and set it to automatically recognize DAutoTest's difs:

<syntaxhighlight lang=bash>
dtest.dlang.io/diff/.*
</syntaxhighlight>

[[File:DAutoTest Git Patch Viewer.png|1200px]]

=== ProjectTester ===

* A couple of selected projects are run to ensure that no regressions are introduced
* Maintainers: [https://github.com/MartinNowak @MartinNowak], [https://github.com/Dicebot Dicebot]

=== CircleCi ===

* Tests are run with code coverage enabled
* Phobos only: Over the last weeks, we have tried to unify Phobos to share a more common coding style. The CI is in place to ensure this status quo for the future
* Phobos only: All unittest blocks are separated from the main file and compiled separately. This helps to ensure that the examples on dlang.org that are runnable and don't miss any imports.
* Phobos only: runs [https://github.com/Hackerpilot/dscanner Dscanner]
* It is [https://github.com/Dicebot/dlangci/issues/18 planned] to move these checks to the ProjectTester

It should be fairly trivial to find out the regarding error here. Just click on the "CircleCi" link and open the red tab that is marked as failing and scroll down to the error message. On a posix system the CircleCi tests can also be executed locally with the <code>style</code> target:

<pre>
make -f posix.mak style
</pre>

== Code coverage ==

If you are too lazy to click on the annotated coverage link, you can install the [https://github.com/codecov/browser-extension browser extension] which will enrich the PR with code coverage information.

Warning: unfortunately `codecov/project` is often showing "random" movement. Please see [https://github.com/dlang/phobos/pull/5202 #5202] for more infos on this.

== Phantom Zone ==

[http://forum.dlang.org/post/ouuutodvhmnghzbeoqen@forum.dlang.org Phantom Zone] is a state assigned to PRs that have value, but would require too much effort from the maintainers to revive without providing enough benefit.
When sending a PR to the "Phantom Zone", please consider the following:

#  Before sending a PR to the Phantom Zone make an effort to invest in the contribution and the contributor.
#  Justify why the PR is being placed in the Phantom Zone
#  Explain to the contributor how the PR can get out of the Phantom Zone

== Saved replies ==

GitHub allows to [https://github.com/blog/2135-saved-replies save replies]. You can set them in [https://github.com/settings/replies your GitHub settings]
Here are a couple of commonly used replies:

=== Missing changelog ===

<pre>
This still lacks a changelog entry. Please see [the changelog folder](../tree/master/changelog) for instructions.
Hence, I added the "pending changelog" label.
</pre>

=== Missing spec PR ===

<pre>
This still needs a PR to the [specification](https://github.com/dlang/dlang.org/tree/master/spec) at [dlang.org](https://github.com/dlang/dlang.org). Hence, I added the label "missing spec PR".

Please refer to the [dlang.org CONTRIBUTING guide](https://github.com/dlang/dlang.org/blob/master/CONTRIBUTING.md) for instructions to build dlang.org locally. If you use Windows, don't worry, you can do your changes "blindly" and preview them at DAutoTest.
</pre>

=== Phantom Zone ===

<pre>
This PR entered the Phantom Zone
-----------------------------------------------


This PR has entered the [Phantom Zone](http://forum.dlang.org/post/ouuutodvhmnghzbeoqen@forum.dlang.org) as it still needs to have the reviewers' concerns addressed and rebased.

Reason for entering the Phantom Zone
----------------------------------------------------

This PR is nice, and normally I would revive such a PR if the author was no longer active. I would also revive it if it were an important bug fix or something of higher priority.  This PR, however, is just a refactoring, so I'm going to put it in the [Phantom Zone](http://forum.dlang.org/post/ouuutodvhmnghzbeoqen@forum.dlang.org) and close it for now.

How do I get this PR out of the Phantom Zone
-------------------------------------------------------------

Easy: Address the comments -> open a new PR (mention this one + short summary in the description).
</pre>

== Milestones ==

* They are intended to show which PRs are basically ready to be shipped OR should be shipped soon
* Please use them whenever you see nothing blocking a PR (except for a final merge decision)
* Ideally about one week before the close of the merge window (i.e. the end of the milestone), the focus on the remaining items of the current milestone should be increased

So far it worked well in the past and present:

* [https://github.com/dlang/phobos/milestone/9?closed=1 2.072]
* [https://github.com/dlang/phobos/milestone/11?closed=1 2.073]
* [https://github.com/dlang/phobos/milestone/12 2.074]

See also the [https://wiki.dlang.org/DIP75 Release process].

== Dlang-Bot ==

The friendly [https://github.com/dlang-bots/dlang-bot Dlang-Bot] is trying to automate boring tasks.

What it does atm:
* Shows whether a PR will be part of the changelog ('X' means NO, '✔' means YES)
* Auto-merges a PR once all required CI pass
* Closes a Bugzilla issue if the respective PR has been merged
* Moves a Trello card if the respective PR has been merged
* Cancels stale Travis builds (this helps to free the Travis queue at dlang/dmd)

What is planned for the future:
* Automatically remove "needs work" / "needs rebase" on a push event
* Recognize common labels in the title (e.g. "[WIP]")
* Automatically tag inactive and unmergable PRs
* Add a "needs review" label to unreviewed PRs with passing CIs
* Show auto-detectable warnings (e.g. regression PR that isn't targeted at 'stable')
* [https://github.com/dlang-bots/dlang-bot/issues <add your issue to the wish list>]

Please see also [https://github.com/dlang-bots/dlang-bot its documentation for a more up-to-date list].

== Changelog ==

There's a changelog folder for all three repos (DMD, Druntime, Phobos).

* The key idea is to have a file per changelog entry, which has the advantage that a changelog can be added to a PR ''without'' creating merge conflicts.
* Take advantage of this feature and don't merge a PR without a changelog entry ;-)
* The separation in the DMD repo between "compiler changes" and "language changes" has been moved, s.t. the "changelog" repo at DMD just contains compiler changes and the [https://github.com/dlang/dlang.org/tree/master/language-changelog changelog at dlang.org] is supposed to contain language changes of the upcoming release.


== Use the formal "Approve" button ==

* GH formalized the review process
* Please use a the "approve" feature instead of LGTM comment

This is important, because all PRs require an approval!
So please approve before you auto-merge (it won't work otherwise).

In the same way GH also allows to attach a "request for changes" on a PR.
If you have a serious remark, please use this "request a change" feature instead of a plain comment as these "request" will be nicely shown as warning at the end of a PR (GH will even block the merge of a PR until the criticized bits are fixed). Moreover "changes requested" will also be shown on the summary of all PRs and helps others when they browse the PR list.

== New name additions to Phobos ==

* All new symbols to Phobos should be pre-approved by Andrei (<code>@andralex</code>). Please request an review from him.
* As Andrei gets quite a load of emails, you should email him directly.

== See Also ==

* [[Starting as a Contributor]]
* [[Contributing to Phobos]]
* [[Contributing to dlang.org]]
* [[Get involved]]

[[Category: Contribution Guidelines]]