[D7] DS Extra Layouts

Looks like there's an empty .gitignore that got accidentally added as part of commit a9c460f

Also, you should fix all of the discrepancies that pareview.sh is reporting. All of the comments that are over 80 chars should be split up into multiple lines.

The reason for this is that the codesniffer tool that pareview uses to verify the coding standard is a file that people *also* use in their editors.

In my editor, I get a slew of "warnings" along side the side of the window indicating problems. While they might not be that big of a deal, I would still have to check and see what kinds of problems they were. https://www.dropbox.com/s/q4hwdem5qai4ghk/Screenshot%202014-05-07%2023.4...

Keeping those things clean means if I ever wanted to submit a patch (say to add another layout) I would need to make sure I didnt introduce any issues myself...

Finally the 'full stop' one is really easy, its just worded silly. Just put a period after the comment, and it makes it compliant.

I didnt actually get to install this, as I dont use display suite, but LGTM. This module will probably save somebody a ton of gruntwork. Nice one.

There's also a lot of comments in the tpl.php files that don't end in full stops.

  <!-- Needed to activate contextual links -->
...
<!-- Needed to activate display suite support on forms -->

Not really a lot else besides, this module is a good idea.

All of the comments that are over 80 chars should be split up into multiple lines.

Im still showing that theres 51 examples of lines over 80 chars. These are all in the comments at the beginning of every file. These are the same errors reported by pareview.sh - so, they still need to be fixed.

Do a search and replace for "String of classes that can be used to style" and replace it with "Classes for styling" and it will cut down *all* of those lines to something that doesn't throw any errors.

Additionally, I just noticed another issue: if you *did* have additional classes, you're going to need a space between the classes and the class thats already there in your template:

So, for example, if $below_right_classes = "customer-area", then the following line:

<<?php print $below_right_wrapper; ?> class="ds-below-right<?php print $below_right_classes; ?>">

will print something like

< div class="ds-below-rightcustomer-area"> 

which ends up with the ds-below-rightcustomer-area being mashed together) so you need a space between the classname and the <?php tag


Finally, *somebody else* needs to actually install this module and make sure it works, as I dont actually use display suite.

Right on. I wasnt sure if that was an issue or not, as I assumed you had it working.

So, now, every file is green in my IDE, no errors whatsoever. All you need now is somebody with display suite to try this out. From a code style and formatting perspective this it totally RTBC.

I installed your module and successfully tested that it's working properly. Simple but useful module.

I've installed this module and tested a a couple of layouts and they seem to work well. Might want to consider RTL language support sometime since it is something ds also offers. Looks good to me.

LGTM based on all the reviews.

I like the project idea, it's useful.
Good project page, well presented.
Thanks for following up the coder review issues. comment length seems trivial, but it's so much nicer and easier to green-light something that gets a 100% on the automated tests.

Behaves as promised. I sort of expected to see some attention paid to margins and other details when you publish a 66/33 layout etc, but that can be refined.
Nothing much that could go wrong with the code. It's a DS plugin that uses the published API correctly and does not introduce any new issues.

Suggest adding references to Panels Extra Layouts also, as it's a direct competitor, due to the way DS re-uses panels layouts automatically.

Based on the above reviews by others your co-operation through the process, clear introduction at the top of this issue, my own visual and (light) testing, and the previous RTBC :

-----------------------

Thanks for your contribution, jeremyr !

I updated your account so you can promote this to a full project and also create new projects as either a sandbox or a "full" project.

Here are some recommended readings to help with excellent maintainership:

• Dries' post on Responsible maintainers
• Best practices for creating and maintaining projects
• Maintaining a drupal.org project with Git
• Commit messages - providing history and credit
• Release naming conventions.
• Helping maintainers in the issue queues

You can find lots more contributors chatting on IRC in #drupal-contribute. So, come hang out and stay involved!

Thanks, also, for your patience with the review process. Anyone is welcome to participate in the review process. Please consider reviewing other projects that are pending review. I encourage you to learn more about that process and join the group of reviewers.

Thanks to the dedicated reviewer(s) as well.