[D7] Front End Developer

There are some errors reported by automated review tools, did you already check them? See http://pareview.sh/pareview/httpgitdrupalorgsandboxjohncionci2221539git

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.

There are still some issues with this project that need to be sorted out.

It appears you are working in the "master" branch in git. You should really be working in a version specific branch. The most direct documentation on this is Moving from a master branch to a version branch. For additional resources please see the documentation about release naming conventions and creating a branch in git.
Review of the master branch:

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

  
  FILE: /var/www/drupal-7-pareview/pareview_temp/css/dfed.css
  --------------------------------------------------------------------------------
  FOUND 6 ERROR(S) AFFECTING 3 LINE(S)
  --------------------------------------------------------------------------------
   56 | ERROR | Style definitions must end with a semicolon
   56 | ERROR | Expected 1 space after colon in style definition; 0 found
   62 | ERROR | Style definitions must end with a semicolon
   62 | ERROR | Expected 1 space after colon in style definition; 0 found
   68 | ERROR | Style definitions must end with a semicolon
   68 | ERROR | Expected 1 space after colon in style definition; 0 found
  --------------------------------------------------------------------------------
  

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.

Source: http://pareview.sh/ - PAReview.sh online service

The CSS errors certainly look like false positives. w3c css validator thinks it's good.

http://jigsaw.w3.org/css-validator/validator?uri=http%3A%2F%2Fdrupalcode.org%2Fsandbox%2Fjohncionci%2F2221539.git%2Fblob_plain%2Frefs%2Fheads%2F7.x-1.x%3A%2Fcss%2Fdfed.css&profile=css3&usermedium=all&warning=2&vextwarning=&lang=en

I guess trying to get some reviews. I'm in the same situation.

Really like it: small, clean and working!

Just a suggestion (I like to optimize code to the max...): change

function dfed_init() {
  if (!path_is_admin(current_path())) {
    drupal_add_js(drupal_get_path('module', 'dfed') . '/js/dfed.js');
    drupal_add_css(drupal_get_path('module', 'dfed') . '/css/dfed.css');
  }
}

to

 function dfed_init() {
  if (!path_is_admin(current_path())) {
    $base = drupal_get_path('module', 'dfed');
    drupal_add_js($base . '/js/dfed.js');
    drupal_add_css($base . '/css/dfed.css');
  }
}

I attached a patch to do this...

Best wishes for your code (I'm on the same boat with a sandbox project of mine).

I've just had a look at this. Seems to work great.

Some notes about the code and some suggestions.

You've used hook_init(). This is going to be deprecated in D8. I would suggest that you should add front end assets through a render array instead. Either with hook_page_alter() or a more specific hook.

Overall though I'd say its fine.

I had a chance to look at this, I would say this is ready to be promoted to a full project.

This has been open for all most two months now... Anyone have any advice on how we can get this promoted?

Don't think it's RTBC yet.

1) As somebody else pointed out in IRC the module's name is not very specific. "Frontend Developer" would make you expect basically everything imaginable, but at the moment it only claims to show the screen resolution. Imho it should reflect this in the module's name. I suggest renaming it to something more specific, no clue what though. :p I know, that this might be my very own opinion.
2) https://drupal.org/node/161085 says the README should be delivered as a .txt file.
3) Typo at: "for theme developers see exactly " .. also copy / pasting, that line doesn't give additional information.
4) The Readme should maybe also include, that you have to give permission to a role to get the module's functionality.
5) It should also include, that it only affects uncached versions of pages and should not be enabled on Production systems.
6) Comment: Implements hook_perm(). has to be "Implements hook_permission()."
7) Comment: Implements preprocess_html(). has to be "Implements hook_preprocess_html()."

@Stefan Lehmann - Thanks for taking a look... I have created issues for 2,3,4,5,6, and 7 above.

Will have to give more thought to number 1. The idea here was to start off with one or two features and then build on the module with community support and feature requests. Similar to the Devel module (http://drupal.org/project/devel). So That's why we named is "Front End Developer" to some day be a toolkit for the Drupal Front End Developer.

@Stefan Lehmann - I resolved the issues above and think this module is ready to be published. Please feel free to review and offer more suggestions.

This seems quite close to https://drupal.org/project/devel_themer -- might it be better to add the features to that?

> * Displays current viewport resolution
> * Displays common media query resolutions ( ie. Mobile, Tablet, Desktop )

Doesn't Firefox's developer mode do that?

> * Highlights links that do not have a destination set

IIRC there is a linkchecker module. That's more about content than theme, surely.

I'd second @nicxvan here - it's a good solid little module and it deserves to be out there in the wild. There's no shortage of modules out there that are slightly overlapping, and this one has some really nice lightweight helpers.

@joachim - the goal with this module is to build in some tools that don't require you use browser tools, which won't help when trying to debug on alternate devices. I don't think this should be a blocker.

One bit of advice might be the name as pointed out above. I'd suggest just making it a tiny bit clearer: Front End Developer Tools (as I know that the intent is to grow over time, but that would be a more descriptive focus)

These guys have been stuck in limbo for a while now and could really use some help getting this promoted to full module status.

Some purely stylistic recommendations: Maybe display a list of all the text at the top, and show the current regions. For example, list out "Mobile Tablet Desktop" and at around 480px have Mobile and Tablet selected to convey that it is within the range of both. You could even make these clickable to automatically jump to Desktop, or Mobile, etc. Also, some way of temporarily hiding it without having to disable the module?

Really liking it. Code is clean and very minimal, and its nice to see some modules with some decent content in the README. Good work.

Hello,

Really simple and good module. Usefull and clean. Just a really little glitch on the readme.txt (line 20):
"How To Use Bloomerang"
What is Bloomerang ?

@churel - Thanks for the note... I have corrected the README file. Bloomerang is another sandbox project I am working on.

Hi.
I'm sorry to say this, but we can't accept this project application because the module is simply to small. See the discussion on how much code do we need to approve a user for ore details.

Code too short

This project is too short to approve you as git vetted user. We are currently discussing how much code we need, but everything with less than 120 lines of code or less than 5 functions cannot be seriously reviewed. However, we can promote this single project manually to a full project for you.

If you would like to promote your project to a full project, simply comment in this ticket.

Thanks.

Hey.

Could you also recommend if maybe this should become a feature of another larger module?

I would suggest creating a patch for linkchecker to include your features to highlight empty links.
Displaying the current viewport/media resolutions would be a possible feature for Theme developer. I must admit, I'm not sure this needs to go into a module at all, since the common browsers are able to display these information using their developer tools or 3rd-party plugins.

@stBorchet Could you please promote this to a full project?

I find that it's helpful when testing on a number of mobile devices and calling out accessibility for images and links.

Oh, sorry. Totally forgot about this.

• The project Front End Developer has been promoted to a full project.
• New URL
  Now that this experimental project has been promoted, you'll need to update the URL of your remote repository or reclone it.
  Don't forget to substitute your project's new short name in the brackets below (and remove the brackets!)
  git remote set-url origin johncionci@git.drupal.org:project/dfed.git

The project page is now https://www.drupal.org/project/dfed.

> Displaying the current viewport/media resolutions would be a possible feature for Theme developer.

This would be the best thing to do, as Theme Developer is already the go-to for this sort of thing.

Closing due to lack of activity. If you are still working on this application, you should fix all known problems and then set the status to "Needs review". (See also the project application workflow).

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