[drupal-devel] E_ALL: first step towards new coding practice.

Gabor Hojtsy gabor at hojtsy.hu
Mon Oct 17 18:49:47 UTC 2005


> Let's start with the most common and uncontroversial ones, 
> and we'll see later what we have left.

Great.

>>2. Decide on better practice (either by 
>>discussing a patch, or a code extract showing the
>>practice)
> 
> Here is a start:
> http://drupal.org/node/34341
> Please comment.

This method of discussion does not scale. Having issues, you exactly
know what status of a modification you discuss (the latest submitted
patch). Having a node which is purposedly about to change does not
constitute a good base for discussion as it renders some comments
obsolote. Also you kick off this node as if you intend to add other
pratices later in there. Expect some issues to heat up (ot least I do so
:), so that you are going to have hunderds of comments on the same node,
hard to filter what comment is about what issue.

Also discussions should not end up in the handbook IMHO. Once we have
the discussed guidelines, then add it to the handbook. This way it looks
quite official, while it is "just" a proposal.

What I would do:

  - start an issue (drupal.org/node/add/issue), per problem type
    (ie. start an issue for the undefined variable notice)
  - attach a sample patch (correct one instance of this error, and
    let the discussion roll around this possible solution)
  - set the issue status to "patch (code needs work)" (actually
    a cross between "patch (code needs review)" and "code needs work"
    would be better, but such status does not exist :)

+ This way you can identify every issue update as belonging to this
piece of coding practice (this is *very hard* with the handbook method
you tried to start with).

+ This way you can show code changes in the form understood by
developers. Patches, yam.

+ This way, it is quite easy to end up in a patch to commit, as you are
already in an issue.

I would name these issues

  "E_ALL: undefined variable/index notices"
  "E_ALL: reference handling notices"
  "E_ALL: ...."

You understand the pattern I guess. This tags these issues somehow, and
makes it easy to find them later.

Note that all issues should end up as a patch for core and as an update
for the coding guidelines (handbook update, patch for any CVS copy).

Goba



More information about the drupal-devel mailing list