4

Context

I had recently come across this question and placed an answer on it, which later on turned out to be wrong as I had misunderstood a small part of the question (I taught he was using node as a cli-client).

After that was brought to my attention I removed my answer, as it wasn't an answer to the question and looked at the other answers and came across this answer.

The problem with the answer

There was however a small problem with said answer. The answer had a small error in it's code which would result in a

Uncaught ReferenceError: tags is not defined" (at least it did in my browser: Chrome v57.0.2987.133 (32-bit)).

Because I agreed with the answer and felt it would be a shame if this small error were to invalidate it I issued an edit. In this answer I change the code so that it no longer produces said error and I change his constants to variables, because it seems like bad practice to use constants at this place (and the edit wouldn't be in compliance with the minimal amount of characters needed to be edited otherwise).

Suggested edit rejected

After some waiting I got the results of the peer review and it turned out that it was rejected. No big deal, the edit could indeed have been issued through a comment. But that isn't what bothers me. What bothers me is that one of the reviewers flagged my edit as

This edit deviates from the original intent of the post. Even edits that must make drastic changes should strive to preserve the goals of the post's owner.

and I find this to be absurd as my edit doesn't cause the answer to deviate from the original posters intent (at all).

Question #1

Why was my edit flagged as

This edit deviates from the original intent of the post. Even edits that must make drastic changes should strive to preserve the goals of the post's owner.

, even though it still conveys the intent of the original poster and only fixes a minor mistake and applies some better practices?

Question #2

Why do the reviewers, that have reviewed my edit, have only a small amount of experience in dealing with JavaScript questions? Shouldn't it be more logical to assign reviewers based on their prior experience with the applied tags?

1
  • IMO fixing an error is fine, but preference-based stylistic changes are not. (I say "preference-based" because e.g. if you're adding indentation to code that has unreadable indentation that's fine, but if you're changing from one readable indentation style to a different one because you don't like it, that's not fine)
    – user253751
    May 10, 2017 at 2:10

2 Answers 2

8

On the whole, your edit was valid. The general guidelines for editing code can be found here:

Do:

  • Test your edited code to make sure it works
  • Fix syntax errors and typos
  • Improve formatting
  • Correct spelling/grammar issues in the answer body

Don't:

  • Change code conventions (delimited_names to camelCase names, etc.)
  • Make the code do something different than what the answer says it does
  • Ignore improvements that can be made outside the code block (spelling, grammar, etc.)

Your edit fixed an obvious syntax error, but it also did make some stylistic changes (const -> let), which I would not generally consider appropriate. Changing const to let does change the functionality of the code slightly (see Choosing how to better take advantage of ES6 const and let)—and remember, const in JavaScript just means that a variable can't be reassigned; it's not immutable even with const, and some consider it better practice to use const instead.

I think there is generally some 'fear' of approving code edits—many people click 'Reject' when the correct choice for them was 'Skip'. Reviewers don't generally see edits on posts that they're familiar with, and they may be presented with an edit to a language that they're totally unfamiliar with. As I mentioned, the correct choice in this situation is 'Skip'—since I know some JavaScript, I would feel reasonably confident in approving your edit, but someone unfamiliar might not notice the difference.

Since your edit was rejected but it was valid, I have manually fixed the part you were trying to address. Sorry that it got rejected in the first place!

As for why anyone can review your edit, not just an experienced user: the vast majority of edits are not on the code, and could be reviewed by anyone. There is some discussion at The Community vs. The Domain Expert, but I believe in most cases, the current system works well enough. The issue here is that reviewers don't know when to click Skip—it's important to remember that you aren't required to review any post, and skipping is a legitimate option, not a bad thing. If domain experts were needed to review each and every edit, the review process would probably take significantly longer, and the edit queue is already pretty long (about 140 at the time of writing, but it often reaches its cap, 200).

The author of the post can override review outcomes, so if they spot an incorrectly rejected edit, they can now approve it after the fact. This should hopefully resolve confusions such as this if the author of the post is active.

8
  • So based on those guidelines my edit could've been rejected, based on the fact that it messes with code conventions? May 7, 2017 at 16:04
  • 1
    Which leaves me with the only reason why I haven't accepted this answer yet: the second question. Why am I getting peer reviewed by users which have less experience, in comparison to me, in dealing with JavaScript answers? Is there a clear-cut reason for this or should I just accept that this is / can be the case and move along? May 7, 2017 at 16:07
  • @DaftKauries Indeed, that could have been one reason. Obviously, I can only guess as to why the reviewers rejected, but my suspicion is that your edit was simply rejected because it changed code, and more than a few people (incorrectly) reject these edits—the philosophy of "never edit code" is far too common. Also, see the latest edit for an answer to your second question.
    – Aurora0001
    May 7, 2017 at 16:11
  • 2
    @DaftKauries how can you tell whether those who reviewed your edit have less experience in js than you?
    – Shadow
    May 8, 2017 at 1:41
  • @Shadow You didn't read my statement correctly. As I have stated multiple times they have less experience in answering JavaScript questions. How can I know this? I have personally looked through their profiles and check the amount of times they have answered on [javascript]-tagged questions and take time to look at some of the questions as one of them had 40+ questions on the tag, but seemed to have primarily asked questions instead of answering them. May 8, 2017 at 9:15
  • 3
    @DaftKauries: yet they could still have more experience with JS, without having had the time to write answers. They could also have had more experience reviewing suggested edits or with SO in general, both of which also apply even if you are not a subject expert. I'm not saying that this is the case here, but using tag answer counts is not a good indicator.
    – Martijn Pieters Mod
    May 8, 2017 at 9:50
  • 2
    @DaftKauries any stats on SO is related to a user's activities on SO alone. You simply cannot tell based on SO activities, who's really experienced in any programming related question. Just check your own profile: you have provided 4 answers with 2 upvotes so far. Shall we conclude that you are not really experienced answering js questions just because of this?
    – Shadow
    May 8, 2017 at 10:41
  • 2
    +100 for encouraging reviewers to skip instead of rejecting in certain cases. May 9, 2017 at 1:48
7

I agree with almost everything that Aurora0001 said in his answer but still want to make another point here:

Although fixing the exception was a valuable edit which I would have approved, I would still have rejected this edit because it also changes const to let without explaining why. The relevant part of the edit comment states

Also don't use const for variables, use let instead.

which seems to be a purely stylistic change (or at least doesn't give any good reason for that). Even after googling about let/const I don't see any clear reason for changing it here.

3
  • 1
    const is for constants, which can't be reassigned, and let is for variables, which should be accessbile on the current scope and child scopes (and child scopes) and can't be reinstantiated. So the difference is not stylistic. May 7, 2017 at 15:27
  • 1
    @DaftKauries: The difference is, in fact, stylistic here. There was no reassignment.
    – Ry- Mod
    May 8, 2017 at 1:41
  • 5
    @DaftKauries if the const to let change is not purely stylistic, then the edit would indeed have to be rejected, since it is not a simple syntax error. If you change anything beyond what is absolute necessary in a code, then there is an icreased risk of rejection and I agree with this approach. I probably would have edited your edit keeping only the syntax error fix.
    – Shadow
    May 8, 2017 at 1:56

You must log in to answer this question.

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