[Setup-tool-hackers] Code conformance

[Hans Petter Jansson <hpj ximian com>]

> 	* Ensure that all 'local' functions ie. ones used internaly in a
> 	module are declared 'static' eg.
> 
> 	static void foo (char *a)
> 	{
> 		fprintf (stderr, "%s", a);
> 	};
> 
> 
> 	* _Please_ try and order static functions where neccessary to
> 	minimise the number of function prototypes eg.
> 
> 	BAD				Good
> 
> 	static int fna (int a);		static int fna (int a)
> 					{
> 	int fnb (int a)				return a;
> 	{				}
> 		return fna (a);
> 	}				int fnb (int a)
> 					{
> 	static int fna (int a)			return fna (a);
> 	{				}
> 		return a;
> 	}

Your mail has points that, being true && common practice, don't call for
explicit agreement. I should add, though, that we're declaring functions
thusly in the XST code:

int
fnb (int a)
{
}

static int
fna (int a)
{
}

With the type on a separate line, that is. That goes for function
declarations - function prototypes and variable declarations are still on
single lines (unless you have to use extra lines to fit arguments, of
course).

I'd also like to take the opportunity to ask for more verbose comments in the
code. They don't increase object size, so bring it on. Also remember that a
comment is a good pre-emptive strike if you're expecting flames :)

I think we should go for this comment style:

Short:

/* So we need to flubar the jalla. */

Moderate:

/* So we need to flubar the jalla. I know this is not a good idea,
 * but for now it will have to suffice. */

(Note ending */ on last line).

Long:

/* Flubar the jalla.
 *
 * Now that we know the prerequisites, we can go ahead and apply the
 * transform. I'd like to get rid of the Z variable here, but fz ()
 * needs it - it would have to be modified to accomodate this. If you
 * come across this comment and it's still not cleaned up, hit me
 * with a stick.
 */

(Note space between summary and description, and ending */ on separate line).

And ideally, we should start documenting the more important functions:

/**
 * xst_ui_coffee_pot_set_enabled: Enable coffee pot.
 * @pot: The CoffeePot to (en|dis)able. @state: Boolean, TRUE means enable.
 *
 * Helper to facilitate easy manipulation of coffee pots. Its native interface
 * is way hairy, and in the common case, we just want to switch in on and off.
 * 
 * Return value: TRUE if the operation was successful, or FALSE if it
 * couldn't be done without blocking.
 **/

gboolean
xst_ui_coffee_pot_set_enabled (CoffeePot *pot, gboolean state)
{
}

(This syntax might seem like overkill, since we're not writing a library as
such, but better safe than sorry - we might want to auto-doc or analyze the
code at some point, anyway).

Next, I'll pick on the grammar and spelling in the comments :)

--
Hans Petter

_______________________________________________
setup-tool-hackers maillist  -  setup-tool-hackers@helixcode.com
http://lists.helixcode.com/mailman/listinfo/setup-tool-hackers