CVS review process, what applicants expect of CVS admins

Those look sane to me. I suggest that we be a bit more permissive of forks/improvements if the author has a solid justification.

Forks could be accepted if there is a solid justification (which means it should not be "I find the module difficult to use"), and if the quality level is at least the same than the original code.

How much solid a justification is risks to be a subjective criterion. As first module, I would expect an original module, in someway; still, I would not want to block too much people from contributing on Drupal.

There are several such modules on d.o, what do you propose to do with them?

Once the author is past the cvs "barrier" they can create any project they want anyway, with or without originality.

It's true that after being approved they can create any kind of projects; still, it would be better if they would not be allowed to create a module/theme by taking code from another module (or making a patchwork with code of different modules) at least while applying to get CVS access.

I also agree to not make the application process to difficult for who wants to contribute; they should, anyway, show they know a minimum of Drupal to write a module as it is supposed to be.

Unfortunately, I think you'll just open a can of worms if you get into the whole "no dupes/forks" thing. I'm to the point where I just don't care anymore. I'm pinning my hopes on the redesign making it easier to sort through the module mess because those of us who think it's better to work together are in the minority.

Otherwise it sounds fine.

Michelle

That sounds better and is esier to apply.

Yeah, I like that better.

Michelle

I think the most important points are already present (security, Drupal API, coding standards); other aspects should be taken in consideration on a case by case basis.

There's to much focus on the coding standards at the moment. This has the following problems:

- finding specks, missing beams
- very negative and tedious for the CVS applicant (and the reviewer I'd think).

Since when is following the coding standards to the dot (literally) a requirement?

Note, if indentation is wrong or the style makes the module effectively unreadable I do think CVS access should not be granted before the style is fixed. A few errors here and there should not be the reason for stalling CVS access. Perhaps just point out the standards, and continue the procedure?

totally agree

The coding standards include also the respect of the module namespace.
There are points in the coding standards that are not merely about the aspect of the code; still, how the code is formatted has its weight, especially because other people could contribute a patch for a problem they found (and trying to fix code that has its particular format is very difficult).

To make a comparison with applying for a driving license, when I take a road test I am valued for many details; when I have a driving license, I can drive using one hand, but that is not something I do on a road test (if I want to have the driving license).
I am not saying that a CVS application has the same importance of applying for a driving license, indeed.

Pointing out to the documentation, and continuing the procedure could not have the desired effect; once obtained the CVS account, it's probable that the applicant forgets what suggested to do (why should he care? He has access to the CVS...).

Maybe I misunderstood the principal points to review in the code, but I remember they were at least secure code, and respect for the coding standards.

I agree minor coding style errors shouldn't hold up an approval, but many times the modules submitted for review are just plain unreadable. Tabs, indentation, etc. I make my best effort to see what's going on, but I cannot give any kind of good review if I can't easily scan the code.

What is about a module that is trying to extend the database API, but it is doing it in the wrong way (with wrong way I mean code that is not using translatable strings, or that is not using placeholders in the SQL query)?

Also, I guess that a module containing code copied from another module present in CVS (and wrongly changed, as when the functions keep the original name) should not be accepted for CVS application.

The book page has been already changed, and it now lists the checkpoints the proposed modules must pass.