I already outlined this in my post to the mailing list, but to recap - In general, the process will be three-passed per pull request.
- Basic formatting to get to PSR-2 compliance (largely automatic)
- In-depth stuff like proper PHPdoc blocks (partly automatic)
- Hunting for bugs or clearing up structures that could be unclear
So the idea with that is that I could roll back to a less controversial commit within a PR and we could already commit that while discussing the stuff that didn't turn out as acceptable.
For the first two passes, it will be a handful of commits each. Third one will be one commit per type of change, or per change if it's important/controversial.
Pull Requests:
- ACL
- attachment_reminder, autologon, database_attachments, debug_logger
- additional_message_headers, archive, emoticons, example_addressbook
- enigma, filesystem_attachments, help, hide_blockquote
- http_authentication, identity_select, jqueryui, markasjunk
- managesieve
- new_user_dialog, new_user_identity, newmail_notifier, redundant_attachments
Notes:
- XML files would also be converted tabs -> 4 spaces
I will ruthlessly convert instances of yoda conditions.-vetoed by Thomas Bruederli- Did not expect to find a .wav file!
- Exploded arrays will be converted into one item per line, according to PSR-2 (just a heads up).
- /tests - Might want to consider using docblocks for explaining stuff instead of just noting what you can already see.
- /tests/Framework - Pretty straightforward stuff.
- /tests/Selenium - ditto.
Separate PRs for:
- /program/include, /program/js (except tiny_mce, of course)
- /program/steps
- /program/localization - mostly just indents
Will skip /program/lib, for obvious reasons.
Notes:
- Includes minified and source version of some scripts. Since you have a global minification available (as far as I can tell), maybe it would make sense to deliver source only and minify on install.
- Wow, there's a lot of PEAR in here. You could make that install via composer as well. In general, converting stuff to a composer-based approach might be a good idea.
- Don't see any updates going on with tiny_mce. Maybe it's better to handle this as a library that is pulled separately instead of including it in the repo? (You could do this via composer as well. If you worry about backwards compat, just use a specific version number.)
Notes:
- Pretty sure /installer/config.php wins some kind of price in terms of technological debt. It will be a pure delight to make that one readable.
- /index.php
- /bin - Not that much to do, there is some inline .php that needs formatting
- /config - same here, just a few indents
- /SQL - mostly just indents
Notes:
- Inline php in .sh files makes it kinda hard for IDEs to figure out the syntax. Might be a good idea to make a /scripts folder that has plain php files that are referenced from the shells scripts.
- The "tt" element is deprecated.
This is mostly css and html cleanup with a little javascript.
Notes:
- There's a number of png files in this, have those been run through pngcrush? (Seems like nope.)
- Javascript formatting would largely follow PHP formatting, not that sure about indents, seeing 2 and four spaces. I guess four is the way to go? (Please don't make me do two spaces.)
- Whew, that's quite a syntax you have there with the custom elements (the stuff). Ever considered switching to something simple like handlebars?
- Lots of CSS as well, maybe LESS/SCSS/SASS should be considered to make these easier to manage. (There are nice PHP libraries that you could call on build.)
Original code guidelines are here.
Some things that were decided along the way:
- 4 spaces indents everywhere (PHP, Javascript, HTML, CSS)
- braces for functions go into a new line, including in Javascript, but excluding inline functions (reference 1, reference 2)