The first two things I think we already agreed on in the comments on GitHub. Will keep an eye out for the variable def blocks in particular.
As for the "last thing" - Yeah that's not acceptable to me. Erasing the history of how I did my work obfuscates my contribution and I would find that rather insulting. I'm happy to contribute and I'm happy to bend my contribution to your specifications. But come on, man, don't mess with the karma aspect of it.
-David
On Wed, Sep 18, 2013 at 11:34 AM, A.L.E.C alec@alec.pl wrote:
On 09/17/2013 05:21 PM, David Deutsch wrote:
Alright, got the first round in rather quickly, Alec commented some more two days ago and I just pushed through a couple more commits.
Some small issues to discuss
- javascript variable initialization with simple assignments:
var a, b, c, d = '', e = [], f = this.env.something;
or
var a, b, c, d = '', e = [], f = this.env.something;
I prefer the latter. "Simple assignments" here are assignments to empty string, array or number.
No new lines in short (2-line only?) code blocks
if (is_array($user)) { $user = array_filter($user); $var = something(); }
In my opinion this is correct formatting. No new line before the second line of the code inside {} brackets when it's variable assignment or break/continue/return statement or unset().
Last thing. After each pull request is accepted we'd like them to be re-created before merge, the way we end up with only one commit per PR.
-- Aleksander 'A.L.E.C' Machniak LAN Management System Developer [http://lms.org.pl] Roundcube Webmail Developer [http://roundcube.net]
PGP: 19359DC1 @@ GG: 2275252 @@ WWW: http://alec.pl _______________________________________________ Roundcube Development discussion mailing list dev@lists.roundcube.net http://lists.roundcube.net/mailman/listinfo/dev