Skip to content

Instantly share code, notes, and snippets.

@daviddeutsch
Last active December 21, 2015 22:29
Show Gist options
  • Save daviddeutsch/6376013 to your computer and use it in GitHub Desktop.
Save daviddeutsch/6376013 to your computer and use it in GitHub Desktop.
Cleanup of RCM codebase

Cleanup of RCM codebase

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.

  1. Basic formatting to get to PSR-2 compliance (largely automatic)
  2. In-depth stuff like proper PHPdoc blocks (partly automatic)
  3. 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.

Rough order of how I will go through this

Plugins (Roughly One PR per Plugin)

Pull Requests:

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 (One PR)

  • /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.

/program

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.)

/installer

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.

Minor Directories (One PR)

  • /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.

Skins classic/larry (One PR per Skin)

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.)

Code Guidelines

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)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment