26
\$\begingroup\$

Note: For chat-like discussions of this subject, let's use this Discuss Close Reasons chat room

Code Review currently has three standard off-topic reasons:

  1. Questions containing broken code or asking for advice about code not yet written are off-topic, as the code is not ready for review. After the question has been edited to contain working code, we will consider reopening it.
  2. Questions must involve real code that you own or maintain. Pseudocode, hypothetical code, or stub code should be replaced by a concrete example. Questions seeking an explanation of someone else's code are also off-topic.
  3. Questions must include the code to be reviewed. Links to code hosted on third-party sites are permissible, but the most relevant excerpts must be embedded in the question itself.

While sites are encouraged to stick to three, we can have up to five standard reasons, if we can justify it. With that in mind, how would you modify the list?

For your reference, here is the relevant excerpt from the Help Center:

To be on-topic the answer must be "yes" to all questions:

  • Is code included directly in my question? (See Make sure you include your code in your question below.)
  • Am I an owner or maintainer of the code?
  • Is it actual code from a project rather than pseudo-code or example code?
  • Do I want the code to be good code? (i.e. not code-golfing, obfuscation, or similar)
  • To the best of my knowledge, does the code work as intended?
  • Do I want feedback about any or all facets of the code?

If you answered "yes" to all the above questions, your question is on-topic for Code Review.

Note that the max-length for a close reason is 400 characters (including markdown).

\$\endgroup\$
  • \$\begingroup\$ Do we also get the option to migrate if we have 5? \$\endgroup\$ – Raystafarian May 5 '16 at 16:15
  • \$\begingroup\$ @Raystafarian Migration is an independent issue from the close reasons. The option to migrate to Stack Overflow continues to exist. \$\endgroup\$ – 200_success May 5 '16 at 16:19
  • \$\begingroup\$ Okay, so with 5 custom we would have 7 options in the off-topic close reason? \$\endgroup\$ – Raystafarian May 5 '16 at 16:20
  • \$\begingroup\$ Follow-up question \$\endgroup\$ – 200_success Jan 12 '18 at 20:07
19
\$\begingroup\$

I found working on this jigsaw puzzle a personally rewarding experience. It really helped me appreciate what you do here.

Obviously the longer your site has been around, the more problems you find that need addressing. I'm sure you've tried cramming more unrelated close reasons into each slot, but that becomes unwieldy fast.

I've been developing a better way of communicating the things that a well-meaning user might think are on topic, but probably aren't. The basic idea is when you start by explaining what the site is about, it becomes way less cumbersome to explain where they went wrong. I've been experimenting with this format, and it's been an unexpected bonus that users seem to come away feeling much more satisfied knowing what the site *is* about… rather than just being told what they did wrong.

Let's see if we can make it work here:

put on hold as off-topic by rolfl ♦, Jamal ♦, 200_success ♦ 1 hour ago

This question appears to be off-topic. The users who voted to close gave this specific reason:

  • Code is not ready to review — Code Review is a community-run website where programmers offer to peer review your fully working code for security issues, readability, completeness, and optimal performance. Unfortunately, questions about non-working or buggy code, incomplete stubs, or code not yet written are outside the scope of this site.

put on hold as off-topic by rolfl ♦, Jamal ♦, 200_success ♦ 1 hour ago

This question appears to be off-topic. The users who voted to close gave this specific reason:

  • Tools and programming concepts are off topic — Code Review is a community-run website where programmers offer to peer review your fully working code for security issues, readability, completeness, and optimal performance. Unfortunately, questions about review tools, methodologies, how code works, adding features, and general programming concepts are outside the scope of this site.

put on hold as off-topic by rolfl ♦, Jamal ♦, 200_success ♦ 1 hour ago

This question appears to be off-topic. The users who voted to close gave this specific reason:

  • Not the owner — For constructive and practical reasons, we ask that all code submitted for peer review be under the direct "ownership" and control of the author requesting the review. Unfortunately, reviewing code written by colleagues, employees, or other third parties is not within the spirit of this site.

Wow. It fits! I hope we can make this work.

This site really pushes the limits of what a Stack Exchange site can do — and you've done a superb job of it! My hope is that we can keep this site approachable and its purpose clear without having to wade through a gauntlet of meta posts and off-topic lists just to know if you are in the right place <fingers crossed>.

Good luck!

\$\endgroup\$
  • \$\begingroup\$ Change is hard, it seems... but, one of the real benefits of the current system is the ability to link to "remedial" links people can follow to get more detail on the close reason motivations, and, importantly, instructions on how to correct their question. Having the same, or similar content for each close reason would make it hard. \$\endgroup\$ – rolfl Jun 11 '15 at 15:04
  • 2
    \$\begingroup\$ Additionally, I am concerned that this information is being presented at the wrong time - closing the barn door after the horse escaped, as such. This information should be presented to users before they ask questions, not after they fail. Improving communication to (new?) users before they ask their first question would be more useful, I think. There is no nice way to tell a person their question is to be trashed after they have asked it.... before they ask it is the time... \$\endgroup\$ – rolfl Jun 11 '15 at 15:07
  • 1
    \$\begingroup\$ @rolfl Fixing the close reasons does not replace any material written before a user asks a question. You can still have your FAQs and meta posts and growing Help Center article. You are asking for more close reasons, but this approach is simply a clearer way to let authors (and everyone looking on) why a question was closed at the source. Not everybody takes on all these reading assignments before they ask a question. I found this way more effective than the tradition approach of piling on more close reasons and cramming in more FAQs that the vast majority of users will never read. \$\endgroup\$ – Robert Cartaino Jun 12 '15 at 16:36
  • 1
    \$\begingroup\$ Thanks for this proposal, which shows it is possible to condense the common rejection reasons into three standard categories. I have difficulty discerning the difference between the first two reasons, in the case of incomplete features, for example. The wordiness and visual similarity is also a concern. We can't apply these reasons verbatim in this current form, but this is a good starting point. \$\endgroup\$ – 200_success May 4 '16 at 21:53
14
\$\begingroup\$

I believe we should use 5 close reasons. This will significantly reduce the confusion people may express when they have to figure out which of multiple close reasons was actually used to close a question:

Note, finding bugs is now in 2, and fixing bugs in 3 - this makes it clear that 2 and 3 could both relate to buggy code, but it is the way the question is asked which makes the 2 or 3 close reason more appropriate.

  1. Questions asking about someone else's code are off-topic because programmers are expected to know what motivated design and implementation choices when presenting code for review. If you don't know why the code is written the way it is, it is assumed that the code is not your own.

  2. Questions asking to explain why code behaves the way it does are off-topic because programmers are expected to be able to explain the code to the reviewers (not the other way around).

  3. Questions asking to fix or add features to code are off-topic because the code is not ready for review.

  4. Questions about hypothetical code, pseudo-code, anonymized code, or anything except the actual code as used in projects are off-topic because context is essential to code reviews.

  5. Questions must include the code to be reviewed. You may reference supplementary code hosted on third-party sites, but only the excerpts embedded in the question itself are in scope for the review.

\$\endgroup\$
  • 2
    \$\begingroup\$ Including the reasons behind the reasons is a nice touch. \$\endgroup\$ – 200_success May 21 '15 at 12:24
  • 1
    \$\begingroup\$ The chosen justification for prohibiting someone else's code could be problematic. "I found this code on someone's blog. The guy advocates Austro-Hungarian notation, because it's smurfy. What do you guys think?" \$\endgroup\$ – 200_success May 21 '15 at 22:04
  • \$\begingroup\$ How do you define "anonymized code"? \$\endgroup\$ – h.j.k. May 23 '15 at 13:02
  • 3
    \$\begingroup\$ It seems to me like (1) and (2) could be merged into a single reason on the single theme of OP not understanding their posted code. \$\endgroup\$ – Nick Udell May 26 '15 at 9:35
  • 1
    \$\begingroup\$ Come and think of it, now (2) runs terribly close to the motivation for our comparative-review tag... perhaps we need to re-word it somehow? \$\endgroup\$ – h.j.k. May 27 '15 at 12:09
  • \$\begingroup\$ I'm uncomfortable with (2) as well. While discussing the closure reasons, we seem to have invented a new off-topic rule. If the new rule is indeed a good idea, we ought to introduce it after thorough deliberation, not by creative interpretation of the "Am I an owner" and "To the best of my knowledge" rules. \$\endgroup\$ – 200_success May 28 '15 at 9:28
  • \$\begingroup\$ @200_success and what is this "new off-topic rule" you say that we have invented? \$\endgroup\$ – Simon Forsberg May 28 '15 at 13:00
  • \$\begingroup\$ @SimonAndréForsberg In the help center, none of the six magic questions explicitly forbids asking for an explanation of working code. \$\endgroup\$ – 200_success May 28 '15 at 15:52
  • 2
    \$\begingroup\$ @200_success If you are seeking an explanation of how your code works, then we will treat the question as if someone else wrote it. - not a new close reason, just not one that was expressed clearly before. \$\endgroup\$ – rolfl May 28 '15 at 16:04
  • \$\begingroup\$ The new way of writing "Explanation of code is off-topic" makes much more sense, really. \$\endgroup\$ – Simon Forsberg May 28 '15 at 17:42
  • \$\begingroup\$ I think we should slightly change the wording of #3 to add something about programming problems (i.e. Stack Overflow-style questions). While it's technically already covered, it'd be more helpful to new users who don't see the connection between "hypothetical code" and "How can I split a string along =?" if they explicitly saw that general programming questions are off-topic in the little close box. \$\endgroup\$ – Nic Hartley Jun 5 '15 at 21:14
  • \$\begingroup\$ I've seen some users not understanding the part about "not ready for review" correctly. That should perhaps be made clearer somehow. \$\endgroup\$ – Simon Forsberg Jun 19 '15 at 14:37
  • \$\begingroup\$ You included a "because" for the first four, but left one out on the last one. Why's that? \$\endgroup\$ – SirPython Apr 28 '16 at 20:39
14
\$\begingroup\$
  1. Code Review is a community where programmers peer-review your working code to address issues such as security, maintainability, and performance. Questions about code that has not yet been written, code that does not work as intended, or code that the author does not understand are off-topic, as the code is not ready for review.

    I've clarified what we mean by "broken", as some users seem to have the mistaken belief that any code that successfully compiles isn't broken. I've added "code that the author does not understand" as a variant of "broken".

  2. Questions must be asked by an author or maintainer of the code and include the code to be reviewed. Links to code hosted on third-party sites are permissible, but the most relevant excerpts must be embedded in the question itself.

  3. Questions must be about real code, not generic best practices in hypothetical situations. Pseudocode, hypothetical code, or stub code should be replaced by a concrete example, since context is essential to code reviews.

The "Do I want this code to be good code?" and "Do I want feedback on all facets?" rules are violated rarely enough that they can be closed with custom reasons instead.

\$\endgroup\$
  • \$\begingroup\$ Posting from my phone... So this'll have to done for now. One reason should cover the general case of the asker isn't even seeking a review. That can include code not yet written or asking for an explanation of code or the results of run code. This separates code not yet written from broken code and let's the broken code reason elaborate on "not working as intended" as well. \$\endgroup\$ – nhgrif May 21 '15 at 11:18
  • \$\begingroup\$ @nhgrif Sorry, I'm not following what you are trying to say. If the asker isn't seeking a review (pretty rare), then Unclear What You Are Asking should cover it? \$\endgroup\$ – 200_success May 21 '15 at 11:22
  • 2
    \$\begingroup\$ "I want to add X functionality." or "I wrote this code, and it works, but why?", and "here is some code I wrote so that you guys don't close it for hypothetical, now let me ask a hypothetical question about best practices that is tangentially related to this question." Importantly, I primarily feel that asking about adding features and asking about fixing bugs are two different things, and the current "broken code" wording tends to result in "but it does work, it just doesn't give the right results always" \$\endgroup\$ – nhgrif May 21 '15 at 11:26
  • \$\begingroup\$ @nhgrif We're definitely getting rid of the word broken, as it seems to cause confusion. "I want to add X functionality" is covered by not yet been written or does not work as intended. "It works, but why?" is covered by code that the author does not understand (which I've recently added). When there is concrete code with a hypothetical question, I would usually edit out the hypothetical part and add a comment saying that we can review the concrete code, but we can't review the hypothetical code that you chose not to post. \$\endgroup\$ – 200_success May 4 '16 at 21:46
  • \$\begingroup\$ So "problem statement" codeless (/clueless!) questions would go under #1, as would "how would I go about implementing X given Y" questions, and of course "where's the bug" questions. And now "why does this code does X" questions formerly under #2... given the overwhelming majority (~65%) of closed questions are closed with the "broken code" reason, I'd prefer to reduce the number of reasons to pick #1, not increase it. But I really like the wordings - I think it's pretty much as good as it gets ... with only 3 custom reasons to play with. \$\endgroup\$ – Mathieu Guindon May 5 '16 at 1:45
  • \$\begingroup\$ @Mat'sMug The way I see it, there's not much distinction between "my code produces wrong results" and "I think my code might be wrong" and "why does this code work?" \$\endgroup\$ – 200_success May 5 '16 at 1:50
  • \$\begingroup\$ How would your close reasons work in the following situations? (1) "What does my code do?" (2) "How could my college improve?" (3) "What dose this code do?" I would assume (1) would be closed under 'close reason 1', (2) under 'close reason 2', but I don't know about (3) it could be either 1 or 2? \$\endgroup\$ – Peilonrayz May 5 '16 at 7:54
  • \$\begingroup\$ @JoeWallis 1) "What does my code do?" → Off-topic: code that the author does not understand. 2) "How could my college improve?" → Unclear what you are asking, if the question makes no sense. Or, if we are asked to review the instructor's code, off-topic: not the author. 3) "What does this code do?" → Off-topic: code that the author does not understand. It could also be off-topic: not the author, if it's clearly someone else's code, but I'd prefer not to make that accusation without evidence. \$\endgroup\$ – 200_success May 5 '16 at 8:01
  • \$\begingroup\$ "or code that the author does not understand are off-topic," I don't like slipping this into the first reason, I think people would forget it's there. Having a 4th close reason that's clear about needing to understand code makes the rule easier to notice and the close reason easier to choose. People have forgotten this rule before because it seemed a bit lost when lumped in with "must be your own code". \$\endgroup\$ – SuperBiasedMan May 5 '16 at 15:44
  • \$\begingroup\$ @JoeWallis Rationale for this grouping is the remediation advice. 1) Fix and verify your code, then come back. 2) Are you the author? If so, post your code here. 3) Stop hiding stuff from us; let's talk about specifics. \$\endgroup\$ – 200_success May 5 '16 at 18:47
4
\$\begingroup\$

Looking through the on-topic questions, I decided to categorize into three different categories, which can be used as close reasons.

I have not added any markdown to the close reasons as I wanted to more or less "throw this out there" first, before focusing on the exact details.

I do like Robert's idea about telling what Code Review as a site is instead of focusing on what is wrong with the question, and I have incorporated this idea in my close reason drafts.

Not something we review

On-topic questions:

  • Is code included directly in my question? (See Make sure you include your code in your question below.)
  • Am I an owner or maintainer of the code?

Close reason:

For constructive and practical reasons, we ask that all code submitted for peer review be under the direct "ownership" and control of the author requesting the review, and that the code is included within the question itself. Unfortunately, reviewing code that is written by a third party or located on a third party site is not within the spirit of Code Review Stack Exchange.

Purpose of reviewing code

On-topic questions:

  • Do I want the code to be good code? (i.e. not code-golfing, obfuscation, or similar)
  • Is it actual code from a project rather than pseudo-code or example code?

Close reason:

Code Review is a community-run website where programmers offer to peer review your fully working code to make it better with regards to security issues, readability, completeness, and performance. To accomplish that we require code to not be stripped of too much context.

(This close reason could probably be improved a bit)

Working, finished code

On-topic question:

  • To the best of my knowledge, does the code work as intended?

Close reason:

Code is not ready to review — Code Review is a community-run website where programmers offer to peer review your fully working code for security issues, readability, completeness, and optimal performance. Unfortunately, questions about non-working or buggy code, or code not yet written are outside the scope of this site.

(taken from Robert's answer, with a minor change)

Not used as a close reason

There is one on-topic question that we tend to ignore when it comes to close reasons:

  • Do I want feedback about any or all facets of the code?

Instead, we do post feedback about all facets of the code, no matter what is being asked.

\$\endgroup\$
  • 2
    \$\begingroup\$ My main concern is the "we require code to not be stripped of too much context" wording, which could be misapplied to shut down any question whose purpose is enigmatic. (That's a quality issue, and should be addressed using downvotes instead.) It's questions about generic best practices that we want to forbid, and the wording needs to reflect that. \$\endgroup\$ – 200_success May 4 '16 at 21:58
  • \$\begingroup\$ My second concern, as with Robert's slate, is the verbosity and visual similarity between the first two reasons. \$\endgroup\$ – 200_success May 4 '16 at 21:59
2
\$\begingroup\$

I think we're mostly discussing how we should arrange the following reasons, from the close-reason meta thread:

I'm of the idea of making them easier for closers to use and to have code not working as intended on it's own. This is as it takes five people to close a single question. (Without moderator intervention.)

I like bold titles such as the ones Robert Cartaino used. They make talking about close reasons easier, I can say 'Code is not ready to review' rather than 'close reason one' for example.

I'd use the following groups:

  1. Owner of code | Explanation of code
  2. Pseudocode, etc. | Code not written yet | No code
  3. Code not working as intended

Why?

  1. Don't know and own the code, Both owner of code and explanation of code link to the same answer in our close-reason meta thread. I also don't like the idea of separating these as I don't particularly wish for us to go on witch hunts to check out if it is someone else's code. It's also more than likely that if you're asking what the code does that you don't own it, otherwise you could read it yourself and after a while understand what it's doing.

    This makes the close reason simple, does the asker say it's someone else's code or ask what it does.

  2. Lacks reviewable code, all have a lack of code. None of pseudocode, stub code, hypothetical code or code not (yet) written have enough code to do a proper review. I'd also include the 'no code' close reason in this section too, it can fit quite well if we phrase it nicely, something like 'or if you forgot to include the code in the question.'

    This would make the close reason simple, if there are no code blocks or if the code blocks are not 'real' code.

  3. Code not working as intended, is our largest close reason, and so having it on its own will allow us to better explain to users what is wrong with their code. Currently our broken code reason confuses new users, so this gives us the option to change the entire close reason to fully and clearly explain what is wrong.

    This will make the close reason simple as if you cannot compile or run the code, it produces incorrect results or it doesn't contain the features the asker wants then it does not work as intended. The same as before and as simple.

I know that I'm going to use a wrong word or some wrong grammar, and so I welcome any improvements to these. These are also heavily based on Robert Cartaino's, as I'm no good at writing close reasons larger that 50 characters. And so I'd word these as:

  • Don't know and own the code - Since Code Review is a community where programmers improve their skills through peer review of working code, we require that all code be posted by an owner or maintainer of the code, and that the poster understands how the code works.

  • Lacks reviewable code - Code to be reviewed must be embedded directly in the question. Note that questions about pseudocode, stub code, hypothetical code, code not yet written, or generic best practices are outside the scope of this site; please post concrete code.

  • Code not working as intended - Code Review is a community where programmers peer-review your working code to address issues such as security, maintainability, and performance. Unfortunately, questions that contain code that doesn't work as intended are outside the scope of this site.

\$\endgroup\$
  • 1
    \$\begingroup\$ 0) Titles (e.g. "Someone else's code") are problematic when reasons are combined. 1) Let's stop saying "broken" because users misunderstand. 2) "Fix a problem" is vague: asking how to fix a scalability problem is OK. 3) How to classify a question that asks how to modify working code to add a feature? If we need to combine reasons, I'd rather not split "code not yet written" from "code not working as intended". \$\endgroup\$ – 200_success May 5 '16 at 18:44
  • \$\begingroup\$ @200_success I've undeleted this now, I'm sure you'll find more problems, so I'll address the old ones again: 0) They should be ok now. 1&2) fixed. 3) I address this in the answer now, it goes against the 'close-reason meta thread' but I think it makes more sense. \$\endgroup\$ – Peilonrayz May 9 '16 at 20:55
1
\$\begingroup\$

It will be good if the off-topic close reasons are ordered such that voters could go down the list and pick the reason that is most likely to stick. (@200_success' idea)

How about something like this (some parts borrowed from other answers):

  • Programming concepts are off topic — Unfortunately, questions about review tools, methodologies, how code works, adding features, and general programming concepts are outside the scope of this site.

  • Not the owner — For constructive and practical reasons, we ask that all code submitted for peer review be under the direct "ownership" and control of the author requesting the review. Unfortunately, reviewing code written by colleagues, employees, or other third parties is not within the spirit of this site.

  • Hypothetical or example code — Questions must involve the real code as it is in your existing project. Pseudocode, hypothetical code, or stub code should be replaced by a concrete example. After that, we will consider reopening it.

  • Code is not ready to review — We offer to peer review your fully working code for good practices, readability, completeness, optimal performance, and security issues. Unfortunately, questions about non-working or buggy code, incomplete stubs, or code not yet written are outside the scope of this site. After the question has been edited to contain working code, we will consider reopening it.

  • Code not included — Questions must include the code to be reviewed. Links to code hosted on third-party sites are permissible, but the most relevant excerpts must be embedded in the question itself. After the code to be reviewed has been included, we will consider reopening it.

A related issue is that the current off-topic close reasons don't line up well with the on-topic checklist. It will be good to adjust those too accordingly.

\$\endgroup\$
  • 3
    \$\begingroup\$ I think we're over-using "we will consider reopening it". \$\endgroup\$ – Simon Forsberg Sep 15 '15 at 19:06

You must log in to answer this question.

Not the answer you're looking for? Browse other questions tagged .