Add panels layouts for the 960gs theme so editors can select a 960gs panel implementation.
Patch on way...

Comments

tayzlor’s picture

StatusFileSize
new58.31 KB

Adding patch. Not sure if it will apply as i created it on a windows pc with cygwin and was having troubles.
attaching anyway for review.

tayzlor’s picture

StatusFileSize
new58.16 KB

new patch...without the packaging information....

tayzlor’s picture

StatusFileSize
new114.67 KB

New patch - should apply properly now.

psynaptic’s picture

Status: Active » Needs work

I tried to do a code review using dreditor but the review was so huge and I don’t want to put you off!

Here is my condensed review which will hopefully make it easier for you to carry out the fixes:

  • There is info added by the packaging scripts in the info file. Please remove this.
  • All files are missing @file directives.
  • Please remove all trailing whitespace.
  • Ensure code comments end with a full stop.
  • Don’t use markup to clear. Use .clear-block on the container instead.
  • There are missing commas off the end of some of the plugin definition arrays.
  • Some plugin definitions have a comment. We should either add a comment to all plugin definitions or remove them all to be consistent. If we add a @file directive with proper description then we won’t need this comment anyway so I recommend removing them.
  • Some files are missing a newline at the end of the file.
  • Indentation of some plugin definitions is screwy.
  • There is a comment above one of the plugin definitions that reads “implementations of hookpanelslayouts”. Please remove this.
  • Do we need to define a CSS file for each layout if it’s only going to be empty?

If there are any of these items that you don’t want to fix, please add a comment explaining your reasoning.