15

Today I've faced two cases where reviewers did not pay attention to reviewed suggested edits.

Review 1

Please, take a look at this suggested edit.

I've thoughtfully read the post, compared as it was before and after edit, and concluded that it became more readable, concise and accurate. I've also noticed that changes do not harm code (#header selector comprises body>#header selector in CSS).
Then I've approved the edit and I was sure that it had to be approved. Soon after that, the edit was rejected due to 3 reject-votes.
Please, judge who is right in this case.

Review 2

Please take a look at another suggested edit.

It is about a wrong tab name. In this case I've looked into IDE settings (I had Android Studio ready to hand) and noticed that the tab is named 'SDK Tools' actually. Path Tools > Android > SDK Manager leads to the following window:

Android Studio settings

I think the editor could confuse this tab with a corresponding section in Standalone SDK Manager that is named 'Extras'. In my opinion these changes harm the answer and I've rejected them.
Then, I have tracked the edit while it has been blindly accepted by three other reviewers and rolled it back.
Sidenote: I've also noticed that none of the three reviewers has experience with Android.


  1. What is the proper behaviour when you notice an incorrect review?
  2. Can the review flow be improved?
  3. Should inattentive reviewers be banned?
18
  • 2
    Answer 1. Flag with custom moderator flag if you notice Incorrect review. 2: Yes it can be improved.(nothing is perfect). 3. There is already banning mechanism for bad reviewers.(i also faced ban).
    – Sandeep
    Jun 11, 2016 at 12:46
  • 25
    @Sandeep - I probably wouldn't recommend a moderator flag for every single review you disagreed with. I'd reserve that for either extremely bad cases (people approving spam or vandalism) or patterns of inattentive reviews.
    – Brad Larson Mod
    Jun 11, 2016 at 13:25
  • 2
    Well in this situation its not just a disagreement @BradLarson OP has a point, and we really can't ignore these faulty reviews that harm a post.
    – Just Do It
    Jun 11, 2016 at 14:30
  • 12
    You are just not reviewing the same way other SO users do it. To get approved, suggested edits need to be compatible with the amount of time reviewers usually spend on a review. Which is 6 to 8 seconds, give or take. The first example is an excellent edit, it just can't be reviewed in 8 seconds so it easier to reject it. Second example can be dusted off in 2 seconds so got approved. The Skip button is a good alternative but doesn't get used enough. Jun 11, 2016 at 14:39
  • So just cause I can't bother to actually review an edit I should just reject it? Interesting @HansPassant
    – Just Do It
    Jun 11, 2016 at 15:13
  • 12
    @JustDoIt No, skip it. Jun 11, 2016 at 15:25
  • 16
    Your first edit was problematic because it actually changed the CSS in a question. That could have been part of the problem; the reviewers have no way of knowing that you are right that the line of code was "unnecessary". Otherwise, your edit was excellent. I would have hit the "improve" button to put that line of CSS back in and left you a comment. But I'm about 10,000 times more conscientious than most reviewers. Which explains why I don't do as many reviews---not enough time! Jun 12, 2016 at 13:43
  • 4
    I think @CodyGray point justifies what happened. I usually Skip if I don't understand the problem but since its common to be reviewing posts that use technologies I'm not familiar with, I'm always a bit apprehensive when someone messed with the provided code (one of the flags actually mentions something like "goes against user intention" or something similar). Personally I refrain from touching the code even if I find it incorrect since these are issues that are to be resolved in actual "answers" or "comments".
    – armatita
    Jun 12, 2016 at 13:51
  • 1
    Your edited phrase "I'm open to any suggestion and recommendation" is incorrect. The original was correct. Jun 12, 2016 at 14:28
  • 11
    I agree with the reviewers that the first one should not have been approved. You should not change code in a question - a bit of white-space cleanup is fine (and appreciated), but changing or removing CSS selectors has the potential to change how the code works, or hide whatever problem the OP was asking about. (In this case it may have seemed that the change wouldn't make any difference, but the OP obviously has other CSS and changing or removing a selector could affect how that rule interacts with other rules because of the way CSS specificity and cascades work.)
    – nnnnnn
    Jun 13, 2016 at 3:42
  • 1
    "What is the proper behaviour when you notice an incorrect review?" I'm sure this question must already have been discussed several times. Jun 13, 2016 at 8:51
  • 2
    @CodyGray If a reviewer doesn't know that the line of code is unnecessary they should have skipped that review.
    – TylerH
    Jun 13, 2016 at 15:38
  • 3
    @TylerH I would agree if the code was in an answer. However, it was in a question. Removing code from a question is not generally a good idea because an answer might be referring to that code. Or you might just be wrong in your assessment. We also do not expect reviewers to be subject experts for every post that they review. If we did, suggested edits to low-activity tags would never get approved. In this case, there is no way to provably know that the line of code is unnecessary; therefore, in your scenario, no one would have ever been able to approve or reject the suggested edit. Jun 14, 2016 at 5:53
  • 3
    @CodyGray, you should not guess if it "might" be referring to anything or "might" be important when reviewing. If unsure - click "skip". That's the only "generally good idea". Jun 14, 2016 at 10:32
  • 3
    No one is talking about guessing! I never used the word "might". Well, I did. I said an answer "might" be referring to it. But that doesn't imply uncertainty. It implies that the reviewers would have to open the question page, and read and understand all of the answers before making a decision. Then hope someone wasn't working on an answer they are about to submit that discusses it, which will now be stale after the edit goes through. No, all of that is ridiculous. Repeating myself: there is no way for any reviewer to be certain that that piece of the CSS was irrelevant to the question. Jun 14, 2016 at 10:59

2 Answers 2

33

I agree with the reviewers on the first post. You removed some of the CSS code. You may have thought it was irrelevant or erroneously included, but in my opinion, that's not a judgement the editor should make; it definitely changes the meaning of the post.

I would also reject the second case, but for the same reason: it changes the meaning of the post. It doesn't really matter whether the answer is correct; the appropriate response, in my opinion, to an incorrect answer is usually either to provide a comment, if you think that will result in a correction, or to downvote and provide a correct answer, rather than to edit the incorrect answer. That leaves the "incorrect" answer there in case others view it as the correct one - in which case my downvote will be outvoted by others' upvotes.

Don't assume that your idea of whether an edit should be approved is always correct.

20
  • 7
    I would argue that in the second case a separate answer is overkill and undermines the efforts of the original answer. A comment "I think you meant the Extras tab" would suffice, IMHO.
    – Ingo Bürk
    Jun 12, 2016 at 0:25
  • 1
    @IngoBürk I agree a comment is a good way of handling it if it's likely the original poster is still around to make appropriate edits.
    – Warren Dew
    Jun 12, 2016 at 0:42
  • 3
    "it definitely changes the meaning of the post." I disagree. Just like how indentation can be freely changed in other languages, with CSS, given that the code should be an MCVE anyway, selectors and trivial things can also be changed. The edit was valid and made substantial improvement to the post.
    – bjb568
    Jun 12, 2016 at 1:43
  • 2
    @bjb568 You and I apparently both missed (at first) that the edit didn't just fiddle around with a selector, but removed a position: fixed property. Given that, it really is a subpar edit. Jun 12, 2016 at 3:56
  • @NathanTuggy But wasn't that redundant? Considering that line was already included for #header.
    – hichris123
    Jun 12, 2016 at 4:35
  • @hichris123: I didn't see it there when I checked. Jun 12, 2016 at 4:36
  • 11
    And don't blame people for "not paying enough attention" just because they disagree with you. That's arrogant and short-sighted. Jun 12, 2016 at 14:27
  • 7
    @bjb568 Just because you think something is trivial, you shouldn't edit out parts of code. There could be something beyond your knowledge that makes that ifo important. I'm not talking about a particular edit, I'm talking in general. We shouldn't encourage editing out code parts. Maybe leave a comment for the OP requesting to clean up the code.
    – T J
    Jun 13, 2016 at 5:45
  • 4
    By the way CSS is a very good example of why you should't edit out parts of the code you think is trivial. What yo think is't making any impact on the browser you're testing could be making a difference in another browser, or another version of the same browser, or in the same browser under different platfrom etc. One can't be an authority in such things.
    – T J
    Jun 13, 2016 at 5:54
  • 3
    @Trilarion Changing the meaning of a post is one of the boilerplate reasons for rejecting an edit, so presumably it's not permitted in edits.
    – Warren Dew
    Jun 13, 2016 at 9:00
  • 1
    @TJ and what if the comment for the OP is hanging in the air for 3 years? (as it is in the first post in review) Jun 13, 2016 at 10:08
  • 5
    @naXa The fact that there is an extra body>#header{position:fixed;} doesn't bother me at all. Like I mentioned earlier, my comments where more general - editing out pieces of code shouldn't be encouraged as a common practice. Maybe in this particular case it wasn't useful, but if people starts editing out code that they think isn't useful from other people's posts without they themselves reviewing it, isn't going to end up well
    – T J
    Jun 13, 2016 at 10:24
  • 2
    @Trilarion whether or not you saw edits removing mistakes being accepted doesn't matter. Such edits shouldn't be accepted, and if they were, it was done so erroneously. As mentioned in this answer, the correct way to address mistakes is with a comment, or providing your own, correct answer.
    – River
    Jun 13, 2016 at 11:10
  • 1
    @naXa The only thing I can think short of editing the error out is providing your own answer then giving attribution where necessary and pointing out the mistake in the other answer too. Jun 13, 2016 at 12:10
  • 1
    @naXa I imagined the case where there is an error in an answer and you do comment on the answer but the original author does not correct the error (for example comment for the OP is hanging in the air for 3 years). Then I would recommend creating another answer (by whomever) which builds upon that answer but not the error. What else do you want to do if editing errors out is forbidden? Jun 13, 2016 at 13:12
3

SO's suggested edit queue is, unfortunately, glutted with careless reviewers. Most of these tend to auto-approve almost anything that's not blatant spam/nonsense, but there's also a smaller yet still significant group of auto-rejecters, although usually these have some particular thing or set of things they react to, rather than rejecting everything they come across.

The first suggestion should have been approved or improved; the only thing it did "wrong" was that it didn't go quite far enough in removing fluff (the request for suggestions at the end is pointless). But given everything else, it was certainly a definite positive improvement. I can only suppose the other reviewers were all careless, saw either large patches of red and green, or back ticks being added, and overreacted accordingly. ("Lyme disease" — use of back ticks for something that's not code — is endemic to SO, but a CSS identifier is certainly code for practical purposes.) For some reason, I (and perhaps you) missed the removal of the first line of CSS properties. That's dead wrong and unjustifiable; the edit should not have been left to stand, but either improved (based on the good points) or rejected and edited (based on the mistake). Changing the semantics of CSS like that would be a bad edit even in an answer, but in a question it's still worse. So the rejection was not actually wrong, but somewhat less than ideal.

In the second case, it would probably have been more efficient to Reject and Edit than to write a custom rejection reason. It wouldn't have taken much longer and would certainly have worked. I can easily see why the reviewers would have mistakenly approved it, but in cases like that, a bit more verification is really called for, rather than blindly trusting a suggester.


For patterns of bad reviewing, or particularly egregious examples, flag a post (ideally one of theirs, but it doesn't really matter) and put all the details into a mod flag and ask them to handle the reviewer's indiscretions. Individual mistakes aren't usually worth worrying about. Generally, mod action will involve either official warnings (which are permanent parts of the user's record, visible to mods) or review banning for a while.

I've previously suggested a cultural adjustment to focus more on suggestions that are at risk of wrong outcomes in review, and Scimonster suggested showing rejection votes when voting to approve.

0

You must log in to answer this question.

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