20
\$\begingroup\$

We have some history, shown below, with our close reasons, there are some problems we have with them. And there has been a consensus to rework them for a while now.

The latest I heard about this was in 200_success' message:

It's time to revisit our standard close reasons. The last initiative failed due to a poor process. I'll think about how to kick off the new discussion.

Since this was posted 2017-11-21, two months ago, I want to know if the moderators, and our community, have thought of ways to tackle the problems with our previous process.


The current close reasons are:

  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 implementation. Questions seeking an explanation of someone else's code are also of-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.

Below is a time line of changes, and requested changes, of the close reasons. They're listed below to make looking up information easier, and so users that haven't followed this before don't have to search meta to understand what's happened, and when.

  1. Revising list of closure reasons

    The close reasons were re-worded to be based on the top voted answer on this question. The close reasons also haven't changed much since. These were implemented on 2014-04-02, with a couple of tweaks to the wording on 2014-04-07.

  2. Rewording close reason “Explanation of code”

    Asks to change the close reason sentence "Questions seeking an explanation of someone else's code are off-topic" to "Questions seeking an explanation of code are off-topic".

    The accepted answer says that's not what the close reason is about.

    The top voted answer says that the wording should be changed.

  3. The close reasons were slightly changed, on 2014-10-16, to have better links to My question was closed as being off-topic. What are my options?

  4. Should we do away with “someone else's code” wording?

    Asks the same as the above. The top voted answer also says this should be changed.

    This however was in favour of revising the standard off-topic reasons.

  5. Should the “broken code” close reason be reworded?

    This says that the current 'groups' of close reasons are confusing. I however can't write a short synopsis.

  6. How should we revise the standard off-topic reasons, if we can have up to five?

    This lists a lot of proposals for how to change the close reasons if you can have three to five close reasons.

    I'm unsure why we didn't change to these close reasons.

  7. [chat] We can't have more than three close reasons:

    Community Managers pushed back. We would have to build a stronger case if we wanted more than three.

  8. Is our “broken” wording broken?

    We came to the conclusion, that whether you agree or disagree that it's broken, using 'code not working as intended' is better than 'broken'.

  9. [duplicate] Can we do away with “someone else's code” wording?

    Closed in favour of completing How should we revise the standard off-topic reasons, if we can have up to five?

  10. [chat] The user that started How should we revise the standard off-topic reasons, if we can have up to five? wants to look into restarting this discussion.

    It's time to revisit our standard close reasons. The last initiative failed due to a poor process. I'll think about how to kick off the new discussion.

If you're 10k you can see when the close reasons were changed. And is how I got some of the above dates.

And so we have the result:

  • Users want to replace "someone else's code" with just "code".
  • We don't want to use "broken code", as it confuses new users. Instead use "code not working as intended".
  • We are limited to three close reasons.
  • We have a lot of possible ways to group the close reasons. We want to avoid a user posting a duplicate answer first, and no-one else getting upvotes - FGITW.
\$\endgroup\$
  • \$\begingroup\$ Do we have a consensus answer yet? It's now a month since this was asked, and there's two good suggestions; I'd like 200_success and Peilonrayz to find a form of words agreeable to both. I would back any suggestion that both those users like. \$\endgroup\$ – Toby Speight Feb 12 '18 at 14:02
18
\$\begingroup\$

As I see it, some of the problems with the existing close reasons are:

  • Ambiguity of "broken code". Some users seem to think that anything that compiles is not broken.
  • Accusations of "not real code". Sometimes, it's hard to tell whether code is real or not. Occasionally, we misjudge, and the author feels insulted by our accusation that their real code is pseudocode. The underlying problem, really, is that their question fails to explain the context in which the code is used.
  • Accusations of "someone else's code". Since "pseudocode" and "someone else's code" are lumped together in the same message, a closure is sometimes misconstrued as an accusation of plagiarism.

In summary, those misunderstandings sometimes cause posters to be frustrated or angry.


These three proposed reasons are largely based on the suggestions by Peilonrayz from the previous discussion.

  1. Authorship of code: Since Code Review is a community where programmers improve their skills through peer review, we require that the code be posted by an author or maintainer of the code, that the code be embedded directly, and that the poster know why the code is written the way it is.

  2. Lacks concrete context: Code Review requires concrete code from a project, with sufficient context for reviewers to understand how that code is used. Pseudocode, stub code, hypothetical code, obfuscated code, and generic best practices are outside the scope of this site.

  3. Code not implemented or not working as intended: Code Review is a community where programmers peer-review your working code to address issues such as security, maintainability, performance, and scalability. We require that the code be working correctly, to the best of the author's knowledge, before proceeding with a review.

Key improvements, relative to the current situation, are:

  • We no longer dedicate an entire reason to "must include the code to be reviewed". It's now combined with the "someone else's code" issue. These two issues may be safely combined without confusion, I think.
  • The standard for "authorship" is more clearly defined and explained.
  • Instead of just banning "pseudocode, hypothetical code, and stub code", we focus on explaining what reviewers are looking for: sufficient context.
  • "Broken code" is more clearly defined.
\$\endgroup\$
  • 4
    \$\begingroup\$ One weakness of this proposal: it is not obvious how a question with no code written yet should be classified — "Authorship" or "Code not working as intended"? \$\endgroup\$ – 200_success Jan 12 '18 at 20:12
  • 4
    \$\begingroup\$ I like this. I however don't fully understand moving 'no code' to 'authorship of code'. However if we stick with selecting from top to bottom, then I think the ambiguity in your comment is fine. \$\endgroup\$ – Peilonrayz Jan 12 '18 at 20:27
  • 1
    \$\begingroup\$ @Peilonrayz I don't think picking a close reason should be order-dependant. That's needlessly complicating things. \$\endgroup\$ – Mast Jan 13 '18 at 8:16
  • 2
    \$\begingroup\$ I don't find no longer dedicate an entire reason to "must include the code to be reviewed" as an improvement with this suggestion. The close reason for "must include the code to be reviewed" is too popular at the moment. Nr 1 seems like 3 close reasons in one. If we could have four close reasons, "must include the code to be reviewed" should have its own reason. Perhaps it's possible to change the wording of your #1 suggestion somehow, I don't know. \$\endgroup\$ – Simon Forsberg Jan 13 '18 at 10:14
  • \$\begingroup\$ @Mast We have sort of ordered them now though. If the code doesn't work, it doesn't matter if you wrote it yourself or if it's example code. If it's example code, it doesn't matter if you've added the code in the question or not. \$\endgroup\$ – Simon Forsberg Jan 13 '18 at 10:15
  • \$\begingroup\$ @SimonForsberg Ordered on severity, yes. IMO that's more intuitive than top-to-bottom on Code Review, even if it's just because we never use the top one (duplicate). \$\endgroup\$ – Mast Jan 13 '18 at 10:45
  • \$\begingroup\$ @Mast When selecting a close reason for the first time, you read each one until you find one that fits. I don't see how it overcomplicates things. If anything it's more intuitive. I could be wrong, and so we can amend the wording at a later date too. \$\endgroup\$ – Peilonrayz Jan 13 '18 at 16:05
  • \$\begingroup\$ @SimonForsberg We're going to have to have one of the close reasons to have three close reasons, if we want broken to be on it's own. 'must include the code to be reviewed' is at 5.71%, so I don't think it's that popular, compared to 'broken code' at 64.91%, which shares the close reason with another close reason. \$\endgroup\$ – Peilonrayz Jan 13 '18 at 16:24
  • \$\begingroup\$ Rev 2 should resolve no-code-at-all questions in favour of the "code not working" reason. \$\endgroup\$ – 200_success Jan 15 '18 at 23:25
  • \$\begingroup\$ Is it possible in reason 3 to nudge people towards Stack Overflow for questions asking for help with known bugs? The standard "Belongs on another SE site" almost does it, but often the question needs refining down to an MCVE before it's suitable over there, and shouldn't be simply migrated as-is. \$\endgroup\$ – Toby Speight Jan 16 '18 at 10:21
  • \$\begingroup\$ @TobySpeight As we do today, each close reason would link to its own explanation page with further advice on what to do. I think that recommending Stack Overflow should be done there, since it's a secondary recommendation, and there are question details that need to be changed before reposting. \$\endgroup\$ – 200_success Jan 16 '18 at 12:35
  • \$\begingroup\$ Ah, I hadn't appreciated that the links would remain - that makes complete sense now. \$\endgroup\$ – Toby Speight Jan 16 '18 at 12:38
  • \$\begingroup\$ I have a couple of last minute possible tweaks (1) Titles should be in title case. (2) In Authorship of Code, "review of working code" -> "review of code". We don't want people to use this instead of 'broken code'. (3) "Code Not Implemented or Not Working as Intended" is long, would "Code Not Working as Intended" work? \$\endgroup\$ – Peilonrayz Feb 16 '18 at 12:44
  • \$\begingroup\$ @Peilonrayz (3) "Code not implemented" guides close-voters choose that reason instead of "Authorship" when the code hasn't been written at all. (2) Agreed, adopted. (1) Capitalizing "Code Not Implemented Or Code Not Working As Intended" would be a bit too heavy for my taste. \$\endgroup\$ – 200_success Feb 16 '18 at 19:40
  • \$\begingroup\$ @200_success I agree with your reasoning. I'll leave it in your hands now :) \$\endgroup\$ – Peilonrayz Feb 16 '18 at 23:00
5
\$\begingroup\$

I mostly agree with 200_success' answer. However there is one close reason I think that should be moved - the no code reason. This is mostly for three reasons:

  1. Not including the code to be reviewed doesn't mean that the user isn't the author. It just means we may violate the licence they use, but relicensing it to CC BY-SA 3.0, if we were to rewrite their code. Something our users do a lot.

  2. If the poster doesn't post the code in the question, then some users may not be able to access the third party site. And so they'd be missing this context. This is common if you're behind a strict corporate firewall. And so the post "Lacks concrete context", or "Lacks reviewable code" as I put it.

  3. Keeping it in the "Lacks concrete context" close reason means that closing questions is simpler. As if any and all problems with code in posts - except code not working as intended - we use this close reason. Where currently, and with the new reason by 200, we'd have to pick one of three close reasons if there is a problem with code in posts.

    This has annoyed me a couple of times before, when selecting a close reason.

And so I'd like to recommend:

  1. Authorship of 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 author or maintainer of the code, and that the poster understands how the code works.

  2. Lacks concrete context: Code Review requires concrete code from a project, with sufficient context for reviewers to understand how that code is used. Pseudocode, stub code, hypothetical code, obfuscated code, and generic best practices are outside the scope of this site. Furthermore, the code to be reviewed must be embedded directly in the question.

  3. Code not working as intended: Code Review is a community where programmers peer-review your working code to address issues such as security, maintainability, performance, and scalability. We require that the code be working correctly, to the best of the author's knowledge, before proceeding with a review.

\$\endgroup\$
  • \$\begingroup\$ Making it simpler to close questions by using an all-inclusive reason is the opposite of what we want to do. The reasons should be as specific as possible, to help inform the poster of what is wrong and how to fix the question. If multiple voters disagree on which reason to pick, so be it — there's no harm in that. \$\endgroup\$ – 200_success Feb 7 '18 at 18:47
  • 1
    \$\begingroup\$ One way to unify the second reason: "Code Review requires contain concrete code from a project, and the code to be reviewed must be embedded directly in the question. Pseudocode, stub code, hypothetical code, obfuscated code, and generic best practices are outside the scope of this site. Furthermore, the question must include contextual information for reviewers to understand how the code is used." \$\endgroup\$ – 200_success Feb 7 '18 at 19:06
  • \$\begingroup\$ @200_success I fail to see how moving the no code close reason creates the opposite of what we want. As we can still "help inform the poster of what is wrong and how to fix the question". I've highlighted what I think are the benefits above. And didn't say anything about "multiple voters disagree on which reason to pick", I just said, IMO, it's easier to close with the correct reason this way, and makes more sense. \$\endgroup\$ – Peilonrayz Feb 8 '18 at 9:10
  • \$\begingroup\$ Minor grammatical edit to item 1 - drop the "Since", and divide into two sentences after "working code": ... through peer review of working code. We require that all code be posted by .... \$\endgroup\$ – Toby Speight Feb 12 '18 at 13:59
  • \$\begingroup\$ Suppose that a question has its code on a third-party site, but would otherwise be a fine question. Which bundled reason would be less inflammatory, if the accusation is misinterpreted? A misconstrued accusation of "insufficient context / pseudocode / obfuscation" would more likely lead to bewilderment and flames than "if you are the maintainer, prove it by embedding the code directly". Under my proposal, the "authorship" reason is about circumstances, and "concrete context" is complaining about the quality of the content. \$\endgroup\$ – 200_success Feb 15 '18 at 21:49
  • \$\begingroup\$ @200_success I understand what you're saying, but I'm lacking the ability to empathize with your point. I guess there are too many unknowns for me. Maybe the above isn't an issue or will add an issue. And so I'm happy to hold off on this idea until we get more of a feel for your close reasons. \$\endgroup\$ – Peilonrayz Feb 16 '18 at 12:08
4
\$\begingroup\$

Just for context and additional information.

The current close-reasons have been used as in the following picture for the last 30 days:

Closure Statistics

Almost 300 questions have been closed, which is not quite 30% of all questions asked.

The overwhelming majority (62%) of these questions are closed as "broken code / code not yet written". After that, there's not quite 20% closed as "Pseudocode / Seeking Explanation".

After that, it's "Unclear what you're asking" with ~7%, "Missing Code" with ~6% with Custom close-reasons and migrations each having 1%.

\$\endgroup\$
  • \$\begingroup\$ Less than 30% closed, is that including or excluding questions removed after closure? \$\endgroup\$ – Mast Jan 13 '18 at 19:32
  • \$\begingroup\$ IIUC this is including questions that are removed after closure. Not that any of the questions counted for this (those of the last 30 days) are eligible for roomba-removal anyways. \$\endgroup\$ – Vogel612 Jan 13 '18 at 19:57
-5
\$\begingroup\$

This is ranging farther afield than just "reword a reason", but I'll throw it out there.

As a reviewer, I would really like to encourage (or make mandatory) automated unit tests that exercise (a portion of) the reviewed code. Think fiddle. Anything the OP does which makes it easier for the reviewer to run the code is a Good Thing.

Soooo, if the "what are my options" FAQ, the "broken code" reason, and the original posting UI mentioned automated tests, and gently nudged OP in the direction of providing tests, that could improve submission quality. Which may bring down the high rate of off-topic posts. This could be phased in one language tag at a time by adding a "-with-test" tag. The site could offer OP immediate concrete feedback about adequacy of his testing, before involving a human reviewer.

\$\endgroup\$
  • 1
    \$\begingroup\$ I think many questions might be improved by additional information regarding the way inputs map to outputs. However, mandatory tests and fiddles is just a way to rationalize closing questions that a particular reviewer is disinclined to answer due to the amount of work involved. It's bureaucracy that would not improve the site. It's ok not to answer a question because it is not worth the effort. It is not ok to close one for that reason. \$\endgroup\$ – ben rudgers Jan 23 '18 at 20:40
  • 1
    \$\begingroup\$ Oh, sorry, there must have been some implied "closing behavior" that I did not intend. I was just going a bit meta, looking at "improved close reasons would improve site quality", and taking that as a jumping off point to "improved submission expectations could improve site quality", in particular making submission expectations more clear could entice more reviewers to devote time to good reviews. \$\endgroup\$ – J_H Jan 23 '18 at 23:12
  • \$\begingroup\$ No need to apologize. I understand where you are coming from. I just tend to oppose taking any StackExchange site in the direction of "increased expectations" and tend to favor "increased charity." Particularly in areas where new users are likely to run counter to expectations and in areas where experienced users may find themselves sometimes peeved...the best solution for peeves is to take a break from the site because it is supposed to be fun. \$\endgroup\$ – ben rudgers Jan 24 '18 at 0:07
  • 1
    \$\begingroup\$ I dislike this suggestion (should be a new question, btw.) for a few reasons: 1. Not all languages have mature unit-testing frameworks 2. Code in languages with mature unit-testing frameworks may not be sufficiently decoupled to write unit-tests, but still be "the best" that OP can write. 3. We're regularly scraping near the upper bound of the character limit on posts (that was explicitly raised for us). Adding Unit-Tests to such large questions (that don't necessarily contain a lot of code) will generally take us over the limit. Questions will get needlessly large... \$\endgroup\$ – Vogel612 Jan 27 '18 at 13:06

You must log in to answer this question.

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