Overview
This module will implement on Drupal the Social Sharing Buttons by GetSocial.io is an all in one toolkit to help you grow your traffic, social shares, followers & conversions. With +15 different apps, we help you grow your website with sharing, following, tracking & engagement apps. Super easy to install, mostly no code required.
This will inject a javascript library to make API calls to the GetSocial API, which will return social bar codes to display in your pages.
Project Page
https://www.drupal.org/sandbox/azeiteiro/2544600
Clone in git
git clone --branch 7.x-1.x http://git.drupal.org/sandbox/azeiteiro/2544600.git getsocial_share_buttons
cd getsocial_share_buttons
Reviews of other projects
https://www.drupal.org/node/2442327#comment-10238149
https://www.drupal.org/node/2491645#comment-10237511
https://www.drupal.org/node/2541320#comment-10236953
--
https://www.drupal.org/node/2554797#comment-10256437
https://www.drupal.org/node/2508285#comment-10256625
https://www.drupal.org/node/2501525#comment-10256677
Comment | File | Size | Author |
---|---|---|---|
#15 | error.png | 123.96 KB | rashid_786 |
Comments
Comment #2
PA robot CreditAttribution: PA robot commentedThere are some errors reported by automated review tools, did you already check them? See http://pareview.sh/pareview/httpgitdrupalorgsandboxazeiteiro2544600git
Fixed the git clone URL in the issue summary for non-maintainer users.
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.
Comment #3
azeiteiro CreditAttribution: azeiteiro commentedComment #4
azeiteiro CreditAttribution: azeiteiro commentedReviewed auto testing tool errors
Comment #5
ARUN AK CreditAttribution: ARUN AK commentedAutomated Review
Fix all PAReview issues http://pareview.sh/pareview/httpgitdrupalorgsandboxazeiteiro2544600git.
Manual Review
Individual user account
Yes: Follows the guidelines for individual user accounts.
No duplication
Yes: Does not cause module duplication and/or fragmentation.
Master Branch
Yes: Follows the guidelines for master branch.
Licensing
Yes: Follows the licensing requirements.
3rd party assets/code
Yes: Follows the guidelines for 3rd party assets/code.
README.txt/README.md
Yes: Follows the guidelines for in-project documentation and/or the README Template.
Code long/complex enough for review
Yes: Follows the guidelines for project length and complexity.
Secure code
Yes: Meets the security requirements.
Coding style & Drupal API usage
(*) Remove Unwanted files and folders from the repository.
(*) Remove .svn folder from lib, and img folders.
(*) If you use variables, you have to erase them on uninstall. You may follow this:
https://www.drupal.org/node/2274697
(+) Please follow drupal coding standards and don't include the js in the below manner:
use drupal_add_js() function to add js.
(+) In hook_menu() you are defining same url(admin/config/services/get_social) two times for change the callback function and you are clearing cache when user submit the registration form.
Instead of defining same url two times, you can do alterations inside callback functions. Based on condition you can alter admin config form instead of re-initialize the admin url. So that you can avoid unnecessary use of drupal_flush_all_caches() function.
(*) Make use of Drupal Behaviours in your javascript code. You can refer JavaScript coding standards
Comment #6
azeiteiro CreditAttribution: azeiteiro commentedFixed all PAReview issues;
Fixed error found in the manual review;
Comment #7
azeiteiro CreditAttribution: azeiteiro commentedFixed all PAReview issues;
Fixed error found in the manual review;
Comment #8
PA robot CreditAttribution: PA robot commentedFixed the git clone URL in the issue summary for non-maintainer users.
I'm a robot and this is an automated message from Project Applications Scraper.
Comment #9
rashid_786 CreditAttribution: rashid_786 as a volunteer commentedKindly abbreviate your module name properly. It should be easy to read and understand.
Refer name convention here https://www.drupal.org/node/299070
Comment #10
azeiteiro CreditAttribution: azeiteiro commentedChanged module name to a smaller one;
Add reviews of other projects;
Comment #11
rashid_786 CreditAttribution: rashid_786 as a volunteer commentedChange your git clone url with update module name.
e.g. git clone --branch 7.x-1.x http://git.drupal.org/sandbox/azeiteiro/2544600.git UPDATED MODULE NAME
cd UPDATED MODULE NAME.
Comment #12
azeiteiro CreditAttribution: azeiteiro commentedChange git clone URL
Comment #13
azeiteiro CreditAttribution: azeiteiro commentedComment #14
azeiteiro CreditAttribution: azeiteiro commentedComment #15
rashid_786 CreditAttribution: rashid_786 as a volunteer commentedAutomated Review:
* Lots of issues found in automated review : http://pareview.sh/pareview/httpgitdrupalorgsandboxazeiteiro2544600git
Manual Review
* Implements hook_help()
It is recommended to get the basic understanding of the module to new users.
* In above code, you are directly embedding external css on the admin page which is taking too much time to load and creating dependency on other server. Is there possibility if you can have those in your module in css directory.
* After installing the module, went to configuration and tried to activate account but found fatal error as in attachment.
Comment #16
azeiteiro CreditAttribution: azeiteiro commentedThank you.
Fixed all errors from manual review.
Automatic review errors:
- Variables are system variables, can't start with module name;
- Since form values are changed via JS on form submit, the way to get tis value is trough $form_state['values'];
Changed to needs review.
Comment #17
azeiteiro CreditAttribution: azeiteiro commentedAdd 3 more manual reviews to speedup review
Comment #18
azeiteiro CreditAttribution: azeiteiro commentedChange applications priority to critical since it has been awaiting response from a reviewer for 4+ weeks
Comment #19
klausiReview of the 7.x-1.x branch (commit a676d9e):
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:
'#markup' => '<p>To get started click on the button below to automatically activate your GetSocial account.</p>',
": all user facing text must run through t() for translation. Also elsewhere, please check all your #markup and #title strings that contain user facing text.font-awesome is a blocker right now.
Comment #20
azeiteiro CreditAttribution: azeiteiro commentedFixed all issues from the manual review.
Fixed errors from the automated tests, expect the following from the DrupalPractice:
The variables used are "site_name" and "site_mail", both are Drupal system variables, not module specific.
This is used to inject javascript in every page on the frontend, so the GetSocial library can be loaded when it is installed. The backoffice JS is added via #attached
The form values are changed via javascript after a Ajax request, so in order to get this new values from the DOM, the $form_state['input'] is needed
Comment #21
klausimanual review:
Otherwsie looks RTBC to me. Assigning to dman as he might have time to take a final look at this.
Comment #22
azeiteiro CreditAttribution: azeiteiro commentedThank you very much for your review, klausi.
About change the javascript load, currently I load it like this:
drupal_add_js(drupal_get_path('module', 'getsocial_share_buttons') . '/js/getsocial_frontend.js');
To load it via page builder array, should I changed it for this code?
$page['content']['#attached']['js'] = array(
drupal_get_path('module', 'getsocial_share_buttons') . '/js/getsocial_frontend.js',
);
Comment #23
klausino objections for more than a week, so ...
Thanks for your contribution, azeiteiro!
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:
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.