Developers of Sympa

[David Verdin <address@concealed>]

Hi guys,

Le 09/11/12 15:07, IKEDA Soji a écrit :

On Fri, 09 Nov 2012 09:36:54 +0100
Guillaume Rousse <address@concealed> wrote:

Le 08/11/2012 21:59, Guillaume Rousse a écrit :

I'd prefer an explicit method call, for better readability:
&mail::set_send_spool(Site->queue());

I prefer to "Site->queue" form, because they seem somewhat
"attributes", not "verbs".  Also, unlike constants with namespace
separator (::), arrow operator (->) doesn't bring syntactic
ambiguity without "()".

Although, I won't stick to this style if people feel difficulty.

I'm not sure I got it correctly but... if it is just a matter of style, I don't care. To get this short: we like people to contribute and will therefore not enforce a particular style, except for engineering purpose; see below.

Actually, I've several similar code style preferences I'd like to 
propose, based on what is usually considered best practices in perl 
community. For instance:

- use direct method call syntax
My::Class->new()

I agree.  "new My::Class()" style is unrecommended and sometimes
doesn't work as expected.

If it's unrecommended and not completely trustable, I completely agree with you.

- use modern perl idiom for heritage:
use parent qw(Parent::Class)
instead of legacy one:
use Parent::Class;
our @ISA = qw(Parent::Class)

parent pragma became a part of Perl distribution at Perl 5.12.0.

So, no. A lot of users probably use older versions of perl. I mean: older packaged versions of perl. We can't force them to install source versions if they don't want.
Also I'm not sure the way I implemented the SDM framework would work as intedended withi this syntax, but I may be wrong.

- use full stricture everywhere:
use strict

I agree.

A noble task which I fully support. It may probably be a hard one, though, especially for wwsympa.fcgi. However, splitting long executables into modules will probably help.

- use modern perl function call syntax:
My::Package::function()
instead of legacy perl 4 syntax:
&My::Package::function()

I agree.

Well, we had this discussion with Marc Chantreux and he was quite eager to switch to the modern form.
With both of you, that make it three skilled perl developper in a row. So, yes, I agree, too... ;)

- use block-based eval syntax:
eval {
     require Foo::Bar;
};
instead of string-based syntax:
eval "require Foo::Bar;"

I agree.

Same here. I don't like the quote form. It is not really usefull anyway.

- use plain english name for magic variables:
use English qw(-no_match_vars)
print $UID
instead of:
print $<

Great!
I hate magic variables in their original form.
Does it prevent contributors from using the ugly form? I'm not sure that would really be necessary...

Etc...

I want to add one thing to the list:

- use context-local variables as filehandles and directoryhandles:
open my $fh, '>', 'file';
print $fh 'blah blah blah';
close $fh;

I may be wrong, but I thought such form woul be the only one allowed in "use strict" context. Isn't it the case?

Thanks for helpful inputs!

What is the best way to first discuss those preferences first, to 
enforce them in the code second ?

To discuss: let's edit this page: https://www.sympa.org/dev/contributors/developers_howto
You can subscribe to this page's modifications notifications by clicking "Subscribe Page Changes" at the bottom. This way, if you take care to let a comment each time you modify the page, we will be able to understand your modifications when we receive the notification (notifications arrive nonetheless, but if you don't let a comment, we just have a diff to understand what you did).
To enforce: we can first create a "first step" by parsing the code and subsituting old snytax with the new one. Later, we can use the Sympa commits hooks to reject modifications that would violate our rules.
About rejection: we should probably select which diverging syntaxes are cxonsidered bad enough to reject a commit. I don't feel like rejecting a commit because the author used spaces instead of tabulations to indent.

Thanks for your proposals!

David

Attachment: smime.p7s
Description: Signature cryptographique S/MIME