63

enter image description here

I've approved this suggested edit yesterday. While it is not a big improvement, at least it did make the types and class names stand out.

And today, I get this message when I try to review :

This was a terrible suggested edit: https://stackoverflow.com/review/suggested-edits/12182586 and should not have been approved. Please don't approve edits like this.

Come back in 3 days to continue reviewing.

Can someone enlighten me, why is this a terrible suggested edit?

22
  • 45
    While in an ideal world you probably should have rejected this edit, I think the ban is a bit overly harsh for this issue.
    – Ctx
    Commented Apr 29, 2016 at 22:24
  • 8
    @Ctx A 3-day review ban really isn't a harsh penalty.
    – Radiodef
    Commented Apr 30, 2016 at 17:52
  • 3
    I also got banned for the same question Commented Apr 30, 2016 at 18:12
  • 6
    A suggested edit which only introduces inline code or emphasis is never useful.
    – bjb568
    Commented May 1, 2016 at 3:08
  • 40
    I can hardly believe that an official Stack Overflow message uses phrasing like that. I would've expected something more like "An edit you recently approved (link) was determined not to meet the quality standards set in the Help Center (link). Please check out the listed standards of a good edit and come back in three days to continue reviewing." Commented May 1, 2016 at 22:57
  • 7
    @Ctx: I understand where you're coming from, but over-zealous reviewers approving everything they see is a huge problem at the moment. If we let every one slide with a "well you shouldn't really have done that, but oh well..." then what's the point in the suggested edits system? Might as well just open it up. If it helps, see the ban as protecting the system rather than punishing the reviewer. After all, it's not like they're actually being locked up for three days in real life. Commented May 1, 2016 at 23:36
  • 7
    pointer array of Service is not code, and neither is segmentation fault, and they should not have been formatted as such. I am otherwise in complete agreement with you and with @TigerhawkT3
    – user207421
    Commented May 2, 2016 at 2:12
  • 7
    If you don't want people to make worthless edits, don't reward them to do so - I say ditch the 2 points low rep users get for editing. I've never thought that was a good idea.
    – Bohemian Mod
    Commented May 2, 2016 at 16:40
  • 3
    @bjb568 Editing is specifically for fixing formatting issues. Not only does it make it easier to read the question, it provides opportunities for new users to understand how they should have formatted the question.
    – Trisped
    Commented May 2, 2016 at 16:45
  • @Trisped Of course. But suggested edits must be substantial and address multiple problems in the post.
    – bjb568
    Commented May 2, 2016 at 17:53
  • 1
    @EJP segmentation fault is output.
    – Random832
    Commented May 2, 2016 at 18:40
  • 1
    @bjb568 Please provide a link detailing that. The only instructions I can find are on the editing page which mentions that only substantial edits should be included, but does not define it. Since the formatting changes above are instantly recognizable and make the post easier to read, I consider them to be substantial.
    – Trisped
    Commented May 2, 2016 at 19:36
  • 2
    @Trisped: Not when the edit makes the formatting worse, no. We don't want users to learn the wrong thing. Commented May 3, 2016 at 21:32
  • 1
    @LightnessRacesinOrbit this conversation was moved to chat. Please observe the rules and continue it there.
    – Trisped
    Commented May 4, 2016 at 18:25

4 Answers 4

39

I triggered this.

The user whose edit you approved had been making a whole ton of awful edits that added backticks to arbitrary phrases and even added signatures to other people's posts!

This was bad enough but the real problem was that a majority of his suggestions were being approved, a clear indication of criminally subpar reviewing that needed to be stopped.

I'd say you got a little unlucky here, being caught up in the clean-up when this edit wasn't as bad as the others. Most of the backticking is borderline acceptable — it's just the "pointer array of Service" and "segmentation fault" that absolutely should not have been backticked. Which does beg the question: what about this edit made you think you should accept it?

The message you're seeing is obviously a manually-constructed message from the helpful moderator who was left with little choice but to "hand out review bans like candy". Since your "punishment" is only a three-day review suspension, I propose that you go out and enjoy the sunshine for a bit, then come back later in the week ready to take a little more care with your reviewing.

7
  • 32
    Comments like this: This was a terrible suggested edit: http://stackoverflow.com/review/suggested-edits/12182586 and should not have been approved. Please don't approve edits like this. Come back in 3 days to continue reviewing. Are pretty rude though.
    – JonH
    Commented May 2, 2016 at 18:43
  • 17
    @JonH: Really? I don't detect any rudeness. It's firm, sure, but that's necessary. We can't go through life expecting butterflies and unicorns at every turn. "Pretty rude" would be "don't you f@cking dare accept a sh!t edit like that again, you little b@stard." I mean, he even said "please"! :) Commented May 3, 2016 at 9:15
  • 5
    @JonH Did you deliberately use code formatting in your comment? Please use code formatting for code and not for normal text. Commented May 4, 2016 at 13:07
  • 1
    @DavidPostill: ahaha Commented May 4, 2016 at 13:47
  • 7
    @JonH - I wrote that ban message, and I stand by it. I think the formatting change made the post worse, and should not have been approved. We have a limited amount of space to write in those, and I needed to convey the link to the edit. I did say "Please". The last sentence is inserted by the system based on the ban term, which I set at 3 days since this was on a Friday and most people don't review over the weekend. I wanted to make sure it was seen. All other reviewers were banned for longer, because they had repeatedly failed audits and clearly were not paying attention.
    – Brad Larson Mod
    Commented May 4, 2016 at 14:36
  • 6
    @DavidPostill Considering there's no way to quote text in comments, and " are not exactly noticeable, code blocks to denote a quote is perfectly fine. There is nothing about the formatting that indicates it's for code other than the name.
    – Rob Mod
    Commented May 5, 2016 at 6:37
  • 1
    @Rob: FWIW I've settled on "formatting like this" which seems to work pretty well. Commented May 5, 2016 at 8:47
102

To expand on Flexo's answer, let's take a closer look at what the specific suggested edit actually changed:

  1. It added backticks around the words "Service", "Court" and "Machine" in this sentence:

    I have abstract class (Service) with two derived classes (Court and Machine).

    These words are (presumably) class names, which are a bit of a borderline case: some users like to always mark them up as code, others prefer to leave them unformatted when they appear merely as names in prose, and not as part of an actual code fragment. Personally, I'd normally respect the original author's choice and not edit them either way, nor would I approve an edit that only added (or removed) backticks around class names. Still, if the rest of the edit was good, I also wouldn't reject just because of a detail like this.

    (The edit also removed the parentheses, which is another stylistic choice. To me, the sentence seems readable with or without them, which again means that I would not personally edit them, but I also wouldn't reject an otherwise good edit just for doing so.)

  2. It added backticks around "a pointer array of Service" in:

    I also have a pointer array of Service, and [...]

    This is unquestionably wrong. The phrase "a pointer array of Service" is not code, at least not in any programming language I know of, but merely a prose description of a data structure. Just adding backticks around the class name "Service" might have been valid (and certainly should have been done for consistency, if the earlier mentions of class names were also backticked), but there's no excuse (other than ignorance or inattentiveness, that is) for marking up "a pointer array" as code.

  3. It added backticks around "if" in:

    I'm using an if.

    This is the only part of the edit that I would consider (marginally) useful, because without code markup, the keyword if looks confusingly like the English conjunction "if", making the sentence potentially difficult to parse. Granted, in this particular case, the meaning seems clear enough even without the backticks.

  4. It replaced quotes with backticks around "segmentation fault" in:

    I receive a "segmentation fault" and [...]

    If you consider the quotes around "segmentation fault" to imply that it's a direct quotation of an error message, then this change might almost be considered valid: one of the valid uses for code markup is for verbatim program output, especially if it contains actual code fragments (like error messages often do) or otherwise is formatted in a way that makes it more readable in a monospace font (and/or as a preformatted text block).

    Still, in the particular case, I don't think that reasoning applies. Not only is "segmentation fault" a perfectly valid phrase in English prose text, describing a certain type of runtime error, but even if it was a direct quote, it still doesn't contain any embedded code or anything else that would benefit from the monospace styling.

It's also worth briefly noting a few things that the edit did not do:

  • It didn't add the missing indefinite article into "I have abstract class [...]".
  • It didn't capitalize (or, for that matter, add consistent backticks to) the class names in "both court and machine".
  • It didn't fix the run-on sentence by (e.g.) replacing the comma after "machine" [sic] with a full stop. (In this case, the comma is clearly wrong both grammatically and stylistically.)
  • It also didn't fix the run-on sentence in the next paragraph (e.g.) by replacing the "and" after "segmentation fault" with a full stop. (This is a more borderline case, since the original is at least grammatically valid, but I'd still consider a sentence break here a significant improvement.)

In short, the edit did (almost) nothing to improve the post, while the changes it did do were either actively harmful (backticking "a pointer array of Service") or were (at best) neutral style changes that disagreed with the original author's perfectly valid stylistic choices. In other words, the edit as a whole was slightly harmful, and mostly useless, with no real redeeming features.

You might legitimately disagree with some of this, if you're one of those people who strongly prefers to see class names marked as code. Even then, though, the suggested edit is at best incomplete, and partly just plain wrong. I could see picking the "Edit" option in review (as Josh KG eventually did, although their further edit really didn't improve things much at all) and fixing the remaining issues with the post, but certainly not accepting the suggested edit as is.

Also, even if you really, really like to use backticks for class names, it's important to note that others do disagree, and that for such legitimately controversial style choices, it's generally best to avoid needless editing and to respect the original author's choice even if it doesn't match your own personal preference.

5
  • 23
    This. Literally the only thing this editor did was putting things in code markdown. And in at least one case the editor was flat-out wrong to do so. This type of editor often does these useless/harmful edits in sprees. Which is one more reason to Reject where you might otherwise Skip - discourage sprees of bad edits. Commented Apr 30, 2016 at 15:48
  • 12
    It also didn't remove the 'Thank you very much :)` noise at the end of the text (but should have done). Commented Apr 30, 2016 at 22:22
  • @JonathanLeffler: Good catch. I only looked at the Markdown diff, which didn't show that. Commented Apr 30, 2016 at 22:45
  • 3
    This is a very well-explained dissection of what went right and what went wrong in the suggested edit. I'm thankful for such dedicated people like you in the SO community
    – Nayuki
    Commented May 1, 2016 at 22:52
  • I agree with basically all of this. The only way this answer fails is that it entirely misses the context of the review ban, which is this. Commented May 1, 2016 at 23:38
56

The edit seems to largely introduce backticks, which are used for code markup for things that aren't code. That's wrong and makes the post worse, not better.

28
  • 51
    In fairness to the OP, about half of the backticks are for object names, which are essentially code, and the others seems to be borderline. And edit ban for something like this seems overly harsh, unless there is more of a history of poor edit approvals. Commented Apr 29, 2016 at 22:08
  • 10
    Isn't it how inlined type name is supposed to be formatted? At least, that is how I've learned from other questions, like : stackoverflow.com/questions/8881291/…
    – Xiaoy312
    Commented Apr 29, 2016 at 22:13
  • 5
    @Xiaoy312 - No. Some people do it out of personal preference but you aren't required to format a class name, keyword, or variable as code in the middle of normal text. Since there are a few users who are known to make edits like this (re-formatting that's nothing but personal preference) in order to troll other users, it's better to reject edits like this.
    – BSMP
    Commented Apr 29, 2016 at 22:21
  • 4
    @Xiaoy312 That would apply to the edits to the first paragraph, but not the edits to the following two.
    – Servy
    Commented Apr 29, 2016 at 22:36
  • 3
    @Servy while I agree that using backticks for the 2nd and 3rd paragraphs are not optimal, the items that did get back ticks were not horrible. One of them items was an operator (an if statement) and an error message and what appears to be a description of an object. It is not a good edit, but hardly one I would say is worth banning someone. Commented Apr 29, 2016 at 23:07
  • 2
    @BSMP edits are not scored at all, they are either approved by the reviewers or not. There are dozens upon dozens bad edits that get approved every day, some are really terrible, and some are just meh. If moderators are going to go and levy manual review bans (and that is what this one was), it shouldn't be subjective - it should be truly terrible. The only part we don't know is if there is a more extensive history of poor edit decisions by the OP that might have justified a ban. Commented Apr 30, 2016 at 0:52
  • 2
    There are exactly zero of these backticks that are valid Commented Apr 30, 2016 at 2:13
  • 16
    "Pointer array of Service" isn't code, but a description of code. That's the deal breaker, where it turns from a pointless edit into a "Lyme disease edit." Either way it should be rejected, it does not improve the post in any way. Commented Apr 30, 2016 at 6:25
  • 35
    @FélixGagnon-Grenier: I very definitely disagree. Every use of a class name in there seems entirely reasonable to me, is how I would format my own post, and I'd be very cross if someone took them out. I agree that "Pointer array of Service" shouldn't have been in backticks, nor "Segmentation fault" but the ones in the very first sentence seem entirely reasonable to me, and helpful. I do find it interesting that the formatting help talks about the mechanics of formatting, but not when it's appropriate. Maybe some community consensus would be useful.
    – Jon Skeet
    Commented Apr 30, 2016 at 6:53
  • 11
    It's official. Jon Skeet has approved the edit suggestion. So cancel the review ban already!
    – Ghasem
    Commented Apr 30, 2016 at 9:17
  • 5
    @JonSkeet exactly - I'd do the formatting you describe. As it stands, some people like putting class names in monospace, and edit them in to make posts clearer, and others don't like it, and ban people for approving edits that do it. BSMP for some reason thinks one of those is trolling and the other isn't. Can't say I agree in the slightest.
    – Rob Grant
    Commented Apr 30, 2016 at 9:26
  • 2
    BSMP for some reason thinks one of those is trolling @RobertGrant - I never said anything like that. I said that there are instances of users using edits to troll others, not that this specific instance was trolling. There's a difference between saying that something happens and saying that something is happening now.
    – BSMP
    Commented Apr 30, 2016 at 20:01
  • 4
    @AlexJolig: That's an appeal to popularity with no basis in logic. Jon Skeet is just a human being, like you and me. Commented May 1, 2016 at 23:35
  • 2
    @user1663987 It would be great if the moderators could leave more personalized guidance when suspending a reviewer. But the sheer size of Stack Overflow doesn't allow that. There are far too many bad reviews every day. That's an old, old problem with review. Commented May 2, 2016 at 9:09
  • 4
    @user1663987: Calling this moderator a "power corrupt dictator" for giving a three-day suspension over a poor edit review is far worse behaviour than anyone else involved in this situation (except, arguably, the original editor). The moderator is literally doing his job, and doing it well. Don't want a suspension? Don't approve harmful edits. Fairly straightforward, that! Commented May 3, 2016 at 9:07
10

Neither answer so far makes an issue of it, but the removal of the parentheses degrades the grammar. If you wanted to remove them and maintain correct grammar, you would have to add articles or other elements. E.g.,

I have an abstract class named Service with two derived classes : Court and Machine.

Or something similar. Just plopping the names in those spots without the parentheses is awkward at best, grammatically incorrect at worst. (I suspect the latter, although I wouldn't be able to cite rules for it.) The parentheses serve to indicate to the read that the sentence is providing extra, not-strictly-necessary information in a non-grammatical place and separates it from the flow of the actual sentence. You might could use commas, but you definitely need something.

4
  • 3
    an should have been there from the start, regardless of the parentheses. Otherwise, a code block signifies the same as the parentheses - there is no real difference. "I have an abstract class Service" is just as (in?)valid as it was with parentheses. If you want to be technical, though, they should be in quotes if we're worried about grammar.
    – Rob Mod
    Commented May 1, 2016 at 12:47
  • 2
    This is not French, so you should use 'THIN SPACE' (U+2009) before the colon (or no space at all). Commented May 1, 2016 at 21:49
  • 1
    @transistor09 I was ensuring the colon would be bolded correctly. =p The space is extraneous.
    – jpmc26
    Commented May 1, 2016 at 22:04
  • I was going to comment on that too (though I would have suggested commas before and after "Service", etc.). I would consider backticks equivalent to quotes, not parenthesis. Commented May 2, 2016 at 11:48

You must log in to answer this question.

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