Semantic UI API [d7]

@inders, please make a git branch e.g. 7.x-1.x, which will let us know which version this is for. And please see the attachment for coder-review.:)

Hi inders

Please check issues reported by PAReview.sh.
http://pareview.sh/pareview/httpgitdrupalorgsandboxinders2201045git

We are currently quite busy with all the project applications and we prefer projects with a review bonus. Please help reviewing and put yourself on the high priority list, then we will take a look at your project right away :-)

Also, you should get your friends, colleagues or other community members involved to review this application. Let them go through the review checklist and post a comment that sets this issue to "needs work" (they found some problems with the project) or "reviewed & tested by the community" (they found no major flaws).

I'm a robot and this is an automated message from Project Applications Scraper.

Please check below to make more clean code.

semantic_ui_api.module

• line 17: Indent should be 2 spaces.
• line 154:
  $variables['attributes']['class'][] = $bundle_settings['color'] . ' ' . 'icon ' . $class;
  ->
  $variables['attributes']['class'][] = $bundle_settings['color'] . ' icon ' . $class;
• line 163-170
  

    $output = theme('html_tag', array(
        'element' => array(
          '#tag' => isset($bundle['settings']['tag']) ? $bundle['settings']['tag'] : 'i',
          '#attributes' => $variables['attributes'],
          '#value' => ' ',
        ),
      )
    );
  

  ->

    $output = theme('html_tag', array(
      'element' => array(
        '#tag' => isset($bundle['settings']['tag']) ? $bundle['settings']['tag'] : 'i',
        '#attributes' => $variables['attributes'],
        '#value' => ' ',
      ),
    ));
  

• line 182-547 Quotes should keep consistency, change double quotes to single quotes.

@inders,

1) Git clone command for the sandbox is missing in the issue summary, please add it.
2) There are still 4 spacing errors reported by PAReview.sh (semantic_ui_api.module : line154) as per drupal coding standards

Really not potential application release blocker(s), leaving you on "Needs review" with the hope that you attend to these.

Review of the 7.x-1.x branch:

• Coder Sniffer has found some issues with your code (please check the Drupal coding standards).
  

  
  FILE: /home/klausi/pareview_temp/semantic_ui_api.install
  --------------------------------------------------------------------------------
  FOUND 1 ERROR AFFECTING 1 LINE
  --------------------------------------------------------------------------------
   5 | ERROR | Doc comment short description must end with a full stop
  --------------------------------------------------------------------------------
  

This automated report was generated with PAReview.sh, your friendly project application review script. You can also use the online version to check your project. You have to get a review bonus to get a review from me.

manual review:

1. semantic_ui_api_init(): why do you need hook_init()? It does not makes sense to run this on drush invocations for example? I think you should rather use hook_page_build() instead when an actual page render array is built.
2. semantic_ui_api_icon_bundle_configure(): the values of $color_options should run through t() for translation. Same for '- None-'.

But otherwise looks RTBC to me. Removing review bonus tag, you can add it again if you have done another 3 reviews of other projects.

Assigning to dman as he might have time to take a final look at this.

The install instructions were a bit off - it seems todays download from the given URL provides a big package that requires some folders to be renamed to match what you describe in the README - but at least the README does describe what the desired state is so I was able to drag the folders around a bit.

Proper use of libraries API, with a requirements check. So OK.

From the project page, it was not clear why I should install this or what immediate benefit it would be. It seems that it should be just a theme-enhancing tool, but on inspection I guess "This module includes some CSS that adds icons to your Drupal links" is about all it does?

In the README - the last step after enabling the module is "your are ready to use Semantic UI !!"
Well, no, I'm not. What button do I press, what config screen can I look at, what does "use Semantic UI" mean at all to a site builder who has just enabled the module?
I can't see the icons yet, though the code contains 400 lines just listing icon names. (Seriously, couldn't you just catalog the available icons from the package somehow?)
Reading the marketing stuff on the semantic-ui.com site tells me nothing about what it means to a Drupal developer. And (on an Omega3 theme) shows no change to the site.

So I'm stuck at an inability to test, because there is no functionality to be found.
It's labelled as an API, so sometimes those can get away without providing any front-end UI ... but then where are the instructions or a usable module that uses the API?

provides a lot of other inbuilt components such as Popups, Form Elements, validations, Buttons, Classic dropdowns, Navigation/Breadcrumb, menus and much more..!

is an empty promise - I can't see that it does any of that at all.

So I enabled icon.module : icon API for block and menu...
That had a settings page /admin/config/media/icon and shows what I guess are the semanticui icons.
.. With that in place, I am seeing and able to use some new icons.

Right - so it seems that either you make icon.module a requirement, or add a bit more explanation on how this is expected to be used. Update the Project page about what it actually does (and doesn't) do and something to the README about how to get started using it - and we'll be good to go.

Apart from what looks to be an inefficient managed list of icon names, the code is fine, so I think we can promote this as soon as the project page is clear about what it's not.

Thanks - I think those extra additions to the project page and the README help a lot, and take this from a "take it or leave it" code dump into a more ready-for-production API that developers can pick up and use. I hope you agree!

Thanks for your contribution, inders!

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.