8

The most recent discussion we had on code review questions was Appropriate Code Review Posts, and the general feel I got from that was "Code review is generally off-topic here, and there is codereview.stackexchange.com, so let's keep code reviews over there".

Personally, I don't care which direction we go in. A case could be made to keep reviews on Codereview, and a case could be made to keep reviews here on SFSE. I do find myself a tad frustrated that suggestions to take it to Codereview never seem to go anywhere.

What I'm looking for here is to set a clear stance on code review questions here on SFSE. Let's pick a path, and stick to it.

Some points to consider:

  • We don't have a big presence on CodeReview
    • This would take time and effort to build
  • Non-mods can't migrate questions to CR at this time
  • SFSE has Salesforce in the name, so traffic for code reviews for code on platform is much more likely to hit here first
  • ...but CodeReview.se has "code review" in the name, so it might just be a matter of discoverability and persistence
  • Our community experts are already here on SFSE
  • Code reviews are probably less likely to help other people than other questions like "how do I deploy to prod using SFDX?" are
    • Unless enough explanation is given and someone is really looking for a better way to do X rather than to get their code working.
  • Some questions like "how do I fix my trigger" end up with answers that kinda straddle the line
    • The answers here are usually "use a trigger framework, and your code should end up looking like XYZ"
  • Going to Codereview if someone's already on SFSE takes clicks, which means some % of askers just won't bother
  • Regardless of the path we take, we should hash out some guidance for when to close poor quality code review questions

A start on guidance for code review questions:

Some of this can be applied in both potential paths (i.e. migrate to CR, or keep on SFSE). Other parts are more geared towards if we decide to accept code review questions here on SFSE.

  • Like Codereview, questions should be closed (or the OP asked to re-focus their question) if the code doesn't currently work
  • Titles should reflect what is being asked (reduce # of queries, modify to use a map, avoid CPU limit, etc...) rather than just "make my code better plz"
  • Provide enough context (code, what the data you're working with looks like, etc...) so that people can understand what the code needs to do
    • I hesitate to say "give us your requirements", because that could lead people to think we will implement their requirements for them
  • Answers should explain why the old approach is bad, why the new approach works, and share details about how it works (i.e. downvote/flag code dumps)
  • Questions should try to focus on a reasonable section of code (which could be a method, or maybe a class or two)
    • We don't want people posting their entire project with 10+ classes and 5000+ LoC
6
  • 1
    My stance hasn't really changed here. Code review questions are extremely difficult to quality control, and inconsistent closure policy may come across as playing favorites. – Adrian Larson♦ Jan 20 '20 at 19:49
  • 1
    I agree that general code review is off topic. I did not initially read this question as a code review question (although I see why you did). It felt specific enough to me to be on topic, but I'm open to pushback on that assessment. – David Reed♦ Jan 20 '20 at 22:46
  • 1
    @DavidReed For me, my code-review senses tingle when the problem statement in the question is (implicitly or explicitly) "how can I improve X?" rather than "getting error X" or "my X isn't working" (which is why I think that question is code review). Taken to extremes, I'm sure a significant fraction of our questions could then be construed as "code review", but it's a starting point. Perhaps that'd be better suited for another meta question. – Derek F Jan 21 '20 at 18:45
  • 1
    If its SOQL DML for loop SF related thing , I do it here on SFSE. If its general code structure, polymorphism related code, i refer it on code-review.SE. – Pranay Jaiswal Jan 23 '20 at 12:31
  • @PranayJaiswal That reminds me that I have written up something I'd like to propose as a canonical-qa for "How do I remove a query from a loop?" – Derek F Jan 23 '20 at 14:11
  • @DerekF that's sounds like a good candidate for canonicalqa. plus few bits like when to use before and when to use after trigger etc. – Pranay Jaiswal Jan 23 '20 at 14:19

You must log in to answer this question.

Browse other questions tagged .