[phpGroupWare-docteam] developers guide mk2

[Dave Hall]

Hi Pascal,

I have read through the new developers guide.  It is a great resource
for new developers.  Here are my comments

Overall

all HTML should be HTML 4 strict compliant

Quotes should be english quotes (")

It would be good if the code examples were captioned and given an
identifier such as Example X.Y (X being the chapter and Y a counter)

Copyright on the coding examples should be 2007-2008 or just 2008
depending on when you wrote them

The code examples should be using the PHP5 OOP syntax, such as
public/protected methods.  The constructor method signature should be
"public function __construct()"

When a method returns nothing then the PHPDoc block shouldn't include a
@return line

1 Introduction
The opening paragraph on page 9 doesn't need "(API)" in the first line.
In the 3rd paragraph, the DB abstraction is "ADOdb derived" and the
templating system should read something like "our user interfaces are
generated using one of 2 templating engines".

The PHPDocs should be descriptive "say_hello method" provides very
little information to another dev.  "Ouputs 'hello world' gives a lot
more info.  The description for constructors should simply be
"constructor"

2.2.1
I have added a little bash snippet to create the directory structure for
people.  Maybe you want to reference it (phpgwapi/doc/new-module.sh).
It is called from the root of a phpgw install like so:
$ phpgwapi/doc/new-module.sh modulename

It is pretty simple and has some basic error checking and document
creation built in

You probably should include something about the coming tests/ dir which
will be used for PHPUnit tests

2.2.2.1 index.php
There is no need to call phpgw::common::phpgw_exit() it is done for you.

2.2.2.2 and other places
the tutorialX_ isn't need in the class name file, this is handled by
phpgw for you.  The code examples target trunk so they should be coded
to work best with trunk.

2.2.2.3
The title array element is ignored in trunk so can be dropped.  You just
need to include an entry in the lang file matching the appname that is
in the common module

the "app_group" element should be tutorial not office

2.2.3
The module shouldn't be installed by dropping some SQL into the db.  The
user should install it properly.  Lets not advocate quick hacks in the
dev guide :)

2.3 "Using the PHPLib based" template system
There should also be a note here that this is the old template system
which is slowly being replaced by the XSLT templating system

I also think we should have an exmaple using XSLT in the dev guide.
Eventually I think the PHPLib guide should be moved at an appendix for
those who really need the info

2.3.2
The template reference should just be hello not hello_file_tpl it adds
too much fluff :)

2.3.4.3 "Adding to your" template file
Should really be using a CSS class and then you can explain the css dir
in the templates :)

2.4 "Using the" database

2.4.1
The db object should be protected.

I think the code example here could do with some work.  Encouraging
users to pull back a full result set to pick a random entry isn't a good
first start imho.

If you do stick with this.  Set the default value of $message at the top
of the method, then you don't need the else in there.   return is a
language construct and so doesn't need the braces "()", just use "return
$message"

2.4.2
The brace for the array should be on a new line

You shouldn't be calling the so layer from the ui layer.  You need a
business logic layer in the middle.  This should also be where the
random selection stuff happens if you keep the example.

2.4.4
I think you really should be using VARCHAR(50) here, there is no need
for TEXT

2.4.5
There seems to be some typos in here.  Also pgsql/mssql need ' for
quoting strings, " won't work.  You will need to swap the quoting used
here.

5.2
Don't concatenate string, just use
$some_value = $this->db->db_addslashes($some_value);
"INSERT INTO some_table(some_field) VALUES('{$some_value}')";

All table names and column follow the same convention as variable names
- no camelCase, use underscores 

There is no need for an else, just return in if block if it fails.  This
creates cleaner code.  The only time you really need if/else is if there
is 2 distinct options and the code will continue to execute for both
code paths.

6.2
I think it would be good to find a real world example where the
positions change.  Is there a good example using en/fr that you can
think of?

Untranslated strings are now unidentified by a exclamation mark (!) at
the start of the string, not an asterisk (*) at the end.  This change
was made to avoid any confusion with mandatory fields.

6.2.2
The description for message_id should specify that it is all in lower
case and a maximum length of 230 characters

The description for content should include that it must use utf8
encoding

"common" is also used for menus, admin and preferences

You might also want to include a note about how easy it is to use OO.o
Calc for maintaining lang files.  It is a simple tool for n00bs to use
for lang files.  (Make sure you mention saving the file with the right
options)

9.1
send::msg() is a pretty crappy example method call. I know it came from
the old doc, but you might want to find a new one.

9.2
The session variable names will change soon, kp3 has been dropped and
phpgwsession replaces sessionid :)

9.3.1
phpgw_header takes an optional argument for rendering the navbar

9.3.3
A new caching system is coming soon, it will use static methods and be a
lot easier to use - not to mention more powerful

9.5
Not very useful imo

9.6.4
charset isn't used in trunk

9.6.5
As much as we hate Microsoft, M$ looks childish.  The order really
should be MySQL, PostgreSQL & MSSQL

9.6.7
NNTP doesn't really belong here

10.1
PHP 5.2
No SAP-DB
The POST requirement should be discussed on the dev list.  I think all
data altering operations should be done via POST, while everything else
should be a GET
E_ALL error reporting on

10.2
Don't duplicate this here.  It is maintained in a text file which can be
opened in any text editor for quick reference.  Adding it here either
makes it a PITA to access (wading through a large document) or a PITA to
maintain (updating it in 2 places)

11.1
appname/inc/functions.incphp Not used with OOPd menuaction and shouldn't
be included here - it is deprecated
same goes for header and footer

11.3/4
These have been replaced by the new menu code.

I think that is it for now :)

Keep up the good work

Cheers

Dave