I think there is a consensus about the requirements for inclusion of a (nontrivial) patch in sympy:

1. All tests have to pass. (Include new tests for new features.)
2. The patch has to be positively reviewed by someone else. (If there a objections, a consensus has to be reached.)
3. Wait at least 24 hours before pushing it in.

I think it should be easily accessible from the website [1], probably from the README too. It should be clear to newcomers that not only contribution is very welcome, but also reviewing patches. (The latter currently lacks somewhat imho.) People new to sympy should not be afraid to review patches.

Maybe there should even be a short how-to about reviewing patches.

What do you think?

[1] http://sympy.org/development.html

There was some talk about not regressing in the coverage_doctest.py test (i.e., all new functions/methods must have a doctest). See issue 294. Is it fair to make that an official requirement before we even have it completely fixed in the existing code? I think we did decide to not make actual code coverage (coverage_report.py) not an official requirement until we get it up in the already existing code (something like 90%).

About the 24 hours thing, I guess it's a good idea. I sometimes don't follow it myself, but it's probably better that way. On the other hand, it might lead to a bunch of positively reviewed patches that need to be pushed in (a problem that we have had a lot of recently). Do you mean 24 hours from the review or 24 hours from the posting of the patch. If it's the former, I think I might have to vote no because of the above concern, but I think 24 hours from the posting of the patch is good, because it gives everyone else, who might be somewhere else in the world and asleep at the moment, a chance to look at the patch and have concerns if they want.

Big +1 on getting people to review stuff. I think some people don't realize that you don't have to have push access to review something, that indeed reviewing a patch and pushing it in are two completely different tasks that can be performed by different people.

Also, people think that we (the main devs) are all experts in SymPy, and are unsure of themselves, when in reality, if person A has made a patch for the geometry code (for example) in the past, and person B has made a new patch for the geometry code, then person A is a better person to review the code than I am, because I have never worked with or even really looked at the geometry code, whereas person A has some kind of knowledge of the code from working with it before. The same is true even if person A has never made a patch for the code but works with it all the time, because again, I never work with the geometry code, so they likely know if the change is a good one or not much better than I would. I use geometry as an example because it's something in SymPy that I have never used and it's an example of something that I see a lot of patches come in for that no one reviews because no one else has used it either (Chris has actually helped this specific problem recently).

We should also make a strong request that people upload their patches as pull requests to GitHub. I think maybe it shouldn't be a requirement, because sometime someone comes along and posts a patch to the mailing list or issue tracker, and maybe if we required that they upload it to GitHub they would just give up and not do it, because they aren't good with git, or maybe they don't have the time, etc. But we should make it clear that having a pull request on GitHub, as well as an issue in the issue tracker if there isn't one already, will prevent the patch from being lost in the cases where no one looks at it for a long time, which happens all too often.

It needs to be in the website, the README, and also in the docs. We need to update http://docs.sympy.org/dev/sympy-patches-tutorial.html anyway (it still references hg!). These days, GitHub has its own little tutorials, so maybe we could just like there for parts of it.

We have a development guide at https://github.com/sympy/sympy/wiki/development-workflow, but it could use a section on reviewing, based on what was said here. Also, it should include a little bit about SymPy-bot.

The page was updated with these points.

We have moved issues to GitHub https://github.com/sympy/sympy/issues.

Migrated to http://github.com/sympy/sympy/issues/5256