Allows the SMS Framework to use phps.kr as a gateway.
Currently only basic sms sending functionality is implemented.
Rules integration is available thanks to SMS framework module.
I just changed project name from SMS PHPSKR to phpskr.

To do:
Check balance and notify shortage to admin user.

Requirement:
Account of korean sms gateway service provider, phps.kr

Dependencies:
SMS Framework

Project page
https://drupal.org/sandbox/mozolady/2092981

GIT
git clone --branch 7.x-1.x http://git.drupal.org/sandbox/mozolady/2092981.git phpskr

PAReview.sh results
All errors and warnings have been fixed.
http://pareview.sh/pareview/httpgitdrupalorgsandboxmozolady2092981git-7x-1x

Sponsored By:
This module is being sponsored by: Next Aeon Inc - a Korean Drupal Shop since 2009

Comments

PA robot’s picture

Status: Needs review » Needs work

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

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.

mozodev’s picture

Status: Needs work » Needs review

Fix the errors and notices listed on the automated review.

mozodev’s picture

Issue summary: View changes

project name & some info updated

gyuhyon’s picture

Tested it and send a sms msg as described

gyuhyon’s picture

Status: Needs review » Reviewed & tested by the community

working as described.

gyuhyon’s picture

Status: Reviewed & tested by the community » Closed (works as designed)

sorry , i was not familiar with the issue queue terms.
this module works as designed for basic function.

klausi’s picture

Status: Closed (works as designed) » Reviewed & tested by the community

This is the correct status, see https://drupal.org/node/532400

kscheirer’s picture

Status: Reviewed & tested by the community » Needs work
  • In sms_phpskr_admin_form(), instead of using a $conf array and setting up defaults, can you use variable_get/set functions instead? You can use system_settings_form() to return the admin form, and then you don't need to store all those values manually.
  • Why are you setting 'msg' => 'test', in sms_phpskr_command() ?
  • There's also no code for case 'getbalance': is that intentional?
  • In case of errors, can you add a call to watchdog() to log failures?

----
Top Shelf Modules - Crafted, Curated, Contributed.

mozodev’s picture

Status: Needs work » Needs review
  • Actually, submit handler of sms_phpskr_admin_form is sms_admin_gateway_form_submit from sms framework module. It sets sms_phpskr_settings variables. So I think that just checking $conf array might be enough.
  • 'msg' => 'test' was just for debugging. Cleaned up!
  • case 'getbalance': Actually I planned to implemnt balance checkingfuctionality next time. I just added.
  • This module returns $result array with false 'status' so that sms framework module can log errors to watchdog. In case sms module doesn't log errors, this module logs errors using watchdog.

Thanks for your review!

Rade’s picture

Status: Needs review » Needs work

Manual review (reading through files, functionality not tested):

  • There is an empty line at the beginning of your .info file, it shouldn't be there.

Otherwise seemed pretty much okay.

klausi’s picture

Status: Needs work » Needs review

That minor issue is surely not an application blocker, anything else? (RTBC is the correct status if you could no find blockers)

mozodev’s picture

Removed empty line at the beginning and polished some code and add some comments to README.txt.. thanks.

mozodev’s picture

Issue summary: View changes

added sponsorship ^^

AjitS’s picture

Priority: Normal » Major
Issue summary: View changes

Two weeks, no review. Changing the priority of the status to Major. I'd suggest you to apply for the Review Bonus so that this issue gets more attention, and someone will look at it right away.

kscheirer’s picture

Priority: Major » Normal
Status: Needs review » Reviewed & tested by the community

You have an unused variable $authkey in sms_phpskr_admin_form(). Could not find any other problems. Pareview's complaint about form_state['input'] is spurious, the value is only being checked, not used.

kscheirer’s picture

Status: Reviewed & tested by the community » Fixed

It's been a month without any problems reported, so I'm promoting this myself as per https://drupal.org/node/1125818.

Thanks for your contribution, mozodev!

I updated your account to let you 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 get 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.

----
Top Shelf Modules - Crafted, Curated, Contributed.

mozodev’s picture

Thanks you all~

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.