3
\$\begingroup\$

Someone might sometimes want a review of a longer length of code: for example this question references some code stored here on their private web site.

It isn't necessarily feasible or desirable to copy too much code into the question text on the main site. (reference)

However if we're to review it then we want some permanent record of it. (reference)

This site already has a Github account. (reference)

Would it be desirable to allow (or require) people, who want a review of a longer bunch of source code, to post their code to Github?

\$\endgroup\$
2

2 Answers 2

3
\$\begingroup\$

I agree entirely with Bobby's answer but wanted to point out a few more things:

  • It's way too easy to put up a lot of code on github, post a link to it on code review and say "plz r3view th1s" (minus the l33t-speak).

    Code Review is already an excellent service that reviews code for free (except for expecting an up-vote or two in return). We should expect something from the users of this site.

    (See also: Questions should include a description of what the code does)

  • Code Review community on GitHub is meant for showing what clean code is, and not for reviewing dirty code. Most of the code publicized on GitHub has already been reviewed on Code Review.

\$\endgroup\$
2
\$\begingroup\$

I think the general consensus has been "if the code is too long to put it into the question, it's too long to be reviewed by us" and I'd like to stick with this.

The problem with allowing to dump GitHub links is:

  • It is not a resource we control, it might be gone one day (repo deletion, user deletion, GitHub blows up...) which would render the answers useless.
  • It allows the users to stretch the definition of "too much code to review" by much. It's hard to argue again someone wanting us to review the whole project instead of a single file.
  • If it is so long that we need to use an external service, the code is really too long, the question should be split up.

At the moment the code that gets posted has a reasonable length most of the time, it's limited to some functions, maybe two or three classes and all in all fits very well. Posting more code would for sure limit the audience of the question ("I'm not going to review all this crap"), so it would be better if the user splits it up and presents it in different questions that can stand on their own (so no "Authentication on website - Part x of y").

If we look at it from the users perspective, being able to dump all code at once on us is a huge advantage. But if we look at it from the perspective of everyone else, having short, concise and self-contained pieces of code, does not only allow the answers to be more specialized, but it will most likely also attract more reviewers (because they ain't need to spent that much time on it) and does also allow us to use the questions for reference purposes in the future.

In short, I think I'd sum it up like this:

First step [for a code review here] is to only show that part of the project that you want to be reviewed, if you already fail at reasonably extracting that part so that it can stand on it's own, you're already doing it wrong.

\$\endgroup\$
3
  • \$\begingroup\$ So in summary, regular users/reviewers here only want to review small blocks of code. \$\endgroup\$
    – ChrisW
    Commented Jan 13, 2014 at 13:45
  • \$\begingroup\$ @ChrisW: Reasonable small, I'd say. Okay, that was made up by me (based upon observations on other SE sites, small questions, low hanging fruits and stuff), but I don't see how reviewing over 10k lines (f.e.) in here might work. The question is very long, the answer gets very long, there's much to point out and fix and so on and on. I don't see how large code would do any good in here. \$\endgroup\$
    – Bobby
    Commented Jan 13, 2014 at 13:53
  • \$\begingroup\$ When I reviewed http://www.mz1net.com/code-sample I confined myself to making general / high-level comments; which I also did in the first version of my review of this question. That (i.e. summarizing) makes it feasible to review larger blocks of code. I can't guarantee anyone would want to, though; and if they do, there are plenty of real-world projects already on Github and elsewhere that people who want to could review, so, probably you're right. \$\endgroup\$
    – ChrisW
    Commented Jan 13, 2014 at 14:01

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