Developers of Sympa

[Guillaume Rousse <address@concealed>]

Le 12/11/2012 15:43, David Verdin a écrit :
> 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.
Having a coding style, even a strict one, doesn't mean refusing 
non-compliants patches. If someone contribute an interesting patch, we 
can pre ou post-process it to achieve conformity ourselve. Let's try to 
be liberal in what we accept, and strict in what we release :)

For this specific case, my point is that the following syntax, despite 
syntactically correct, is less immediatly recognisable as the call of a 
method on a object:
$object->send
by opposition to
$object->send()

And attribute accessors are methods. BTW, I also prefer the explicit 
usage of get/set methods, precisely to keep methods named after verbs, 
but that's a different issue :)

Ok, let's forget about those two issues right now, if they don't make 
consensus.

[..]
> > > - 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.
My fault, I didn't checked availability first. Let's use base instead, 
available since 5.4:
use base qw(Parent::Class);

> Also I'm not sure the way I implemented the SDM framework would work as
> intedended withi this syntax, but I may be wrong.
It doesn't change anything, it's merely syntactic sugar (see man base 
for details).

> > > - 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.
I'm even more ambitious here, I plan to someday have all executable use 
the taint mode, for enforced security :)

> > > - 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.
It my be useful, when you construct the string dynamically. Which is not 
the case here.

> > > - 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...
No, as said before.

> > > 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?
If you're refering as the usage of 'my' in front of the variable 
declaration, you're correct. But if you're refering to the usage of 
variable ($fh) against typeglobs (fh), you're not, they are perfectly 
legal in strict mode. Hence the interest of variables here.

BTW, the Soji proposal actually covers two aspects:
- use the 3-parameters form for open(), meaning the mode is the 2nd 
parameter, and the filename the 3rd
- use local variables, not typeglobs, for file/directory handles

[..]
> > > 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).
I can subscribe to the page, I can't modify it.

> To enforce: we can first create a "first step" by parsing the code and
> subsituting old snytax with the new one.
OK, but we now have 3 development branches. Is it worth to apply those 
changes separatly, or should we rather do it in one place only, to 
minimize the work ? I lack experience of merging branch with subversion, 
so I don't have a clear vision here.

> 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.
Well, some things are simple to do via commit hooks, and others less. 
More-over, it's often time-consuming, and frustrating when wrong. So, 
for everything we are discussing right now, I'd rather try to define a 
shared Perl::Critic configuration 
(http://search.cpan.org/~thaljef/Perl-Critic-1.118 for details), and 
rely on manual change review (what I'm currently trying to do), or 
test-triggered problem detections, than commit hooks.

Also, you're leveraging the next discussion point on my list: 
indentation policy. And it seem we have opposition opinion here, as I 
hate tabs :)

More precisely, my own usual practice is to use 4-spaces indentations 
steps, and wrapping long lines to 78 characters, but I'm open to 
alternate proposals, as long as we fix the current ugly mix of 
tabulations and spaces.

--
Guillaume Rousse
INRIA, Direction des systèmes d'information
Domaine de Voluceau
Rocquencourt - BP 105
78153 Le Chesnay
Tel: 01 39 63 58 31

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