28

This suggested edit made one change to the question -- changing the title to ALL CAPS. I rejected it with a custom note and was pleased to see that the review was rejected by other reviewers as well.

However, out of curiosity, I looked at the other suggestions made by the user and ran into this.

It is evident that the user has removed all the import statements from the code and the review was happily approved by three reviewers. I don't quite agree that the review should have been approved. Of course, I can rollback and make revision to the post in order to fix it but have some questions:

  • Does a mechanism exist for flagging such reviews?
  • Shouldn't reviewers be given a break for permitting such changes?
11
  • 18
    That's so comically bad
    – random
    Commented Oct 14, 2013 at 5:53
  • 3
    I've seen another review where the title had a shell variable (in all caps, like PATH), and someone tried to change it to lowercase because "yelling hurts my eyes". Also went through (before I could reject it). Commented Oct 14, 2013 at 5:53
  • 14
    Why are people changing code in a question?
    – user102937
    Commented Oct 14, 2013 at 5:54
  • 3
    do we need tougher edit audits? Commented Oct 14, 2013 at 5:57
  • Can someone able to do so roll back the edit on this question by that user? I've flagged it for attention, too. The only change was to put the question in a quote block. (Apparently this is a pattern for this user: 1 2, plus the edit in the question which was disastrous) Commented Oct 14, 2013 at 5:59
  • 6
    What's with all the blockquote prefixes in that suggestion? o.o It's even weirder that two of these three users also have extensive Reject histories and I don't understand why on Earth they would approve that suggestion of all of them.
    – animuson StaffMod
    Commented Oct 14, 2013 at 6:00
  • 2
    I am literally getting a headache reading through many of these logs. This cannot be acceptable.
    – Jamal
    Commented Oct 14, 2013 at 6:16
  • 2
    Some people believe that import statements aren't important, I expect they will have been involved in approving this edit. I strongly disagree Commented Oct 14, 2013 at 7:24
  • 1
    And the robo-reviewers stike again. Commented Oct 14, 2013 at 8:18
  • @RobertHarvey It seems that people continue to change code, and suggested edits continue to get approved.
    – devnull
    Commented Oct 14, 2013 at 15:08
  • @devnull: That one is just changing variable names, but it could be argued as too minor. I see it wasn't unanimously approved.
    – user102937
    Commented Oct 14, 2013 at 17:40

2 Answers 2

19

While it is debatable (and it has been debated here many times) whether or not changing code in an answer is acceptable, there is almost zero justification for changing code in a question. You always run the risk of invalidating the OP's question.

As to the specific edit that was done, there isn't much point to it. I am aware that the custom is to not include imports or using statements in code posted here, but omitting them can actually make more work for novice users, who now have to figure out which libraries are being used in the code.

You can flag the affected post. Use a custom flag, and describe the problem in detail.

8
  • 2
    An experienced Java eye can determine which import statements are unusual and which are run-of-the-mill. If I were tidying up a question, I would consider removing standard imports to neaten or shorten a long code sample. Of course, adding block quotes throughout the code sample is just... strange. Not to mention the other edits performed by that user. Commented Oct 14, 2013 at 7:20
  • 6
    @Duncan removing imports seems controversial, see here for example Commented Oct 14, 2013 at 7:28
  • My thoughts on this seem to be unpopular based on past posts, but nonetheless: I disagree with the claim that ... there is almost zero justification for changing code in a question. You always run the risk of invalidating the OP's question. This might be true for "help, my code doesn't work!" questions, but as long as the question is well-written and you understand it thoroughly, it simply isn't always the case. If you can clearly see that an error in the code is incidental to the author's problem (but is perhaps a nuisance when trying to reproduce it), then I think it's right to fix it.
    – Mark Amery
    Commented Oct 14, 2013 at 19:17
  • Of course, I agree that the edits in this particular case are bad, though.
    – Mark Amery
    Commented Oct 14, 2013 at 19:20
  • "I am aware that the custom is to not include imports or using statements in code posted here" -- Is it? Is that a Java-specific thing? That seems to conflict with the advice given by sscce.org. Commented Oct 14, 2013 at 19:34
  • 1
    @KeithThompson: I base my statement on my own observations. I rarely see code samples that include the necessary import statements.
    – user102937
    Commented Oct 14, 2013 at 19:37
  • 2
    @MarkAmery: You can disagree all you like, but if someone flags your edit that removes includes as invalid, as a mod I'm likely to side with the flagger.
    – user102937
    Commented Oct 14, 2013 at 19:38
  • 1
    @Mark, Here is a faq-proposed on when to make edits to code. For questions, as Robert pointed out, code should not be edited for content.
    – jmac
    Commented Oct 15, 2013 at 0:54
10

Actually, the first suggested edit in all likelihood did not change the title to all caps. It was already in all caps in the original post, and the suggested edit merely failed to fix it. Check the timeline:

This seems to be a bug: the suggested edit was accepted for submission even though the post had been edited since the original version. Since all of this happened in the first 5 minutes of the post, it's possible that a non-recorded edit by the original poster muddied matters further.

Suggested edit diffs are wonky when there are concurrent edits, you can't set store by them.

For the second suggested edit, the timeline is:

The first suggested edit retained the > marks left over from the quotation formatting in the original, presumably because Monica had started editing from the original and not from the result of edit #1. Leaving the > marks was an error, and could have been a valid rejection reason, however the suggestion was not the vandalism that it looks like at first glance. Again, the suggested edit should have been rejected by the system due to another edit having taken place.

As for removing import statements, it has been suggested but turned down. This would be a valid reason to reject with a custom message.

1
  • If a user suggests an edit and then another 2k user submits an edit started before the suggestion then the suggestion is rejected. If a user submits a non-suggested edit while someone is in the editor suggesting a new edit, and they suggest it after the other edit is make, then it will simply appear that the suggester is "undoing" the previous edit, rather than preventing them from making it to begin with. That seems like the best you can really do in that situation. If I realize that I try to reject with a custom reason along the lines of "conflicts with a concurrent edit".
    – Servy
    Commented Oct 14, 2013 at 18:36

You must log in to answer this question.

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