The first three stages of the DIP review process allow members of the D community to provide feedback on a DIP before it reaches the Formal Assessment stage. A DIP that enters the review pipeline should emerge before the Formal Assessment as polished and solid as possible. It's helpful to all involved, but particularly the DIP author and the DIP manager, if the community reviewers remain on topic and keep their feedback on target. Adhering to the following guidelines will assist in achieving both goals.
Feedback discussions take place in GitHub Pull Request threads for the Draft Review stage and in forum threads for the Community and Final Review stages. Two forum threads will be initiated for the latter two stages: a discussion thread and a feedback thread.
The discussion thread is for freeform discussion of the proposal. Commenters are encouraged to discuss the proposal's merits, implementation details, alternative approaches, etc. This thread is subject to normal forum moderation practices, though completely off topic posts are more likely to be deleted.
The sole purpose of the feedback thread is to solicit critiques of the DIP contents. No discussion is allowed. The following rules will be enforced in the feedback thread:
- All posts must be a direct reply to the DIP manager's initial post, with only two exceptions:
- Any commenter may reply to their own posts to retract feedback contained in the original post
- The DIP author may (and is encouraged to) reply to any feedback solely to acknowledge the feedback with agreement or disagreement (preferably with supporting reasons in the latter case)
- Feedback must be actionable, i.e., there must be some action the DIP author can choose to take in response to the feedback, such as changing details, adding new information, or even retracting the proposal.
- Feedback related to the merits of the proposal rather than to the contents of the DIP (e.g., "I'm against this DIP.") is allowed in Community Review (not Final Review), but must be backed by supporting arguments (e.g., "I'm against this DIP because..."). The supporting arguments must be reasonable. Obviously frivolous arguments waste everyone's time.
- Feedback should be clear and concise, preferably listed as bullet points (those who take the time to do an in-depth review and provide feedback in the form of answers to the questions in this document will receive much gratitude). Information irrelevant to the DIP or is not provided in service of clarifying the feedback is unwelcome.
Posts in a feedback thread that do not adhere to the above rules will be deleted at the DIP manager's discretion.
The remainder of this document provides guidelines for contributing useful feedback in a DIP review.
It is important that feedback stays on topic. Too often, feedback threads are filled with personal opinions that are not aimed at strengthening the DIP, or devolve into off-topic discussions. Such discussions increase the burden on the DIP author who must monitor the discussion for actionable feedback, for the DIP manager who must monitor the discussion (in Community and Final reviews) to extract a review summary, and for anyone trying to follow the discussion with a focus on the quality of the proposal. The DIP manager will strictly ensure that review threads remain on topic.
Posts that do nothing more than declare the commenter's opposition to the DIP, e.g. "I think this is a terrible idea and there's no way I'll support it", are off topic. No one is seeking your support for the DIP. What we want is actionable feedback. If you have concrete reasons for why the proposal is a bad idea or is severely flawed, list them. Then your comment will be on topic and the author will have some potentially valuable feedback to consider. The DIP manager will delete such posts that lack any valuable feedback and paste them in a new post in a separate DIP discussion thread.
Posts that wander off on the pros and cons of something completely unrelated to the quality of the DIP, such as the joys of string handling in C or the incompetence of computer science professors or some ridiculous issues with one OS or another, are very off topic. The DIP manager will delete such posts with both prejudice and pleasure.
In short, reviewers are asked to keep all of their comments focused on identifying weaknesses in and providing suggestions to improve the DIP, and to avoid corrupting such useful feedback with extended expositions that are off topic. The DIP manager and the DIP authors will thank you.
The Draft Review, Community Review, and Final Review stages of the DIP process all serve different purposes with their own set of guidelines, but a common set applies to both.
The DIP process is intended to be collaborative, not adversarial. The goal is to work together as a community to produce the best possible outcomes for the D language. The language maintainers are not your enemy, neither are the DIP author, the DIP manager, nor other members of the community. All involved want what is best for the language, though they will often disagree on how to get there. Remember that before posting and endeavor to keep your posts free of personal insults and ad hominem attacks.
The DIP process is neither a popularity contest nor a public vote. The ultimate goal of each review stage is to strengthen the proposal and to clarify risks and benefits by improving the DIP's language and clarifying its technical issues. There are two broad questions on which each reviewer should focus their consideration and aim to provide detailed feedback.
Before considering the technical details of the proposal, the reviewer should consider its merit. This is a subjective question for which providing concrete guidelines is likely not possible. A sample of possible points to consider:
- does the proposal actually improve the language, i.e. it isn't simply bikeshedding or change for change's sake?
- does the perceived benefit of the proposal outweigh any complexity it may add?
- is the potential for code breakage acceptable?
- is there a possible alternative to implementing the proposed feature as a library rather than as a language change?
Consider a proposal to change the immutable keyword to imm. While some may consider such a change beneficial for the number of keystrokes the new keyword would save, there is a strong argument to be made that such a change is unacceptable on the grounds that it falls in the category of bikeshedding. On the other hand, a proposal to replace the body keyword with do brings an obvious benefit in reducing the number of keywords by one, ultimately presenting a minor reduction in the complexity of the language at no cost.
If, as a reviewer, you find yourself with any doubts as to the acceptability of a DIP, please make a note of your concerns in the Draft or Community review stage. Such feedback is less useful in the Final Review stage.
This is the question that delves into the technical details. Not all reviewers will have the requisite background to answer the question outright, but it can be broken down into more specific components that reviewers of varying skill levels can attempt to evaluate.
- is the proposed feature specified in sufficient detail? A DIP should provide enough detail that a programmer with the requisite skill set to implement the proposed feature, and should provide examples demonstrating each unique aspect of the feature in action. It's conceivable, even likely, that a DIP author has not considered every possible angle in drafting the proposal. In the worst case, the detail may be insufficient to the degree that a top to bottom read of the DIP leaves the reviewer uncertain as to what is being proposed. More realistically, reviewers may notice gaps in the proposal for which they can make suggestions to provide clarity.
- are edge cases, flaws, and risks identified and addressed? Beyond the specification of the proposed feature, the DIP author must also consider and attempt to detail the potential side effects of its behavior. No DIP author possesses a crystal ball and, no matter their level of knowledge and experience, cannot be expected to foresee all possible side effects. Reviewers should draw on their own experience to uncover any issues the author may not have addressed. It's particularly important to consider if the proposed feature opens any security holes in the language or conflicts with existing language features.
- are there any platform or architecture pitfalls to be aware of? While this plausibly falls under the previous category, it is worth emphasizing. LDC and GDC support a broader range of architectures than DMD, and this should not be forgotten by the DIP author. Reviewers should consider any potential for platform- or architecture-specific issues that the author may have overlooked.
- is there an implementation that proves the proposed feature works in practice? A DIP is not required to be accompanied by an implementation in order to leave the Draft Review stage, but the presence of an implementation will strengthen the DIP. Not all DIP authors will have the means to implement their proposals, but an attempt should be made to find someone who can. If there is no implementation, why not? Was the feature too difficult to implement or was there simply no one available with the time and means to implement it? Reviewers with the time and means might consider volunteering to provide an implementation in that case. If there is an implementation, reviewers are encouraged to test the examples provided in the DIP and push further by experimenting.
- does the DIP consider prior work from other languages? Rare are the language features that exist in a vacuum. The DIP author should identify other languages including the same (or similar) feature, provide a comprehensive review of the pros and cons of the implementation(s), and a comparison of each instance with the current proposal. As a reviewer, you should draw on your own experience with other languages and identify any prior work you find missing from the proposal.
- if the proposed feature is a breaking change, is there a well-defined migration path? Breaking changes can be controversial, but debates on their merit or value are not on topic in a DIP review. However, examination of the DIP's Breaking Changes and Deprecations section is on topic. Reviewers should consider if the author has identified all potential breakage and clearly outlined a migration path that accounts for each.
Any other weaknesses or potential improvements a reviewer identifies in the proposal are fair game for feedback.
Both of the first two stages of the review process are aimed at improving the DIP's language (see the author guidelines for hints), its technical strength, and its overall quality. Personal opinions on the merit of the proposed feature are welcome in both stages as long as they are accompanied by concrete reasons that identify flaws with the proposal.
The goal of these review stages is to strengthen and to clarify the risks and benefits of the proposal, not to shoot it down or discourage the author. Discussions about specific aspects of the DIP are welcome in the discussion thread as long as they are focused and do not devolve into endless arguments; such discussion is not allowed in the feedback thread. By the time a DIP has gone through its final Community Review round, it should be in or extremely near a final draft form.
Feedback in this stage is intended to uncover any flaws that were missed in the previous review rounds. Ideally, a DIP in Final Review will require no revisions. In this stage the feedback thread is not the place for personal opinions, with or without concrete reasons. Nor is it the place for suggesting fundamental changes to the DIP unless they are motivated by the discovery of a fundamental flaw. Take such opinions and feedback to the discussion thread instead.
In short, if you are unable to identify any problems with the DIP, then please do not comment in the Final Review feedback thread.