Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add a lot of documentation, documentation script, update class docblocks #9

Open
wants to merge 8 commits into
base: master
Choose a base branch
from

Conversation

delatbabel
Copy link

I updated class docblocks for all of the gateways including links to web sites where available. I also added a simple script to create the API documentation, might be useful to some people while developing. No functionality changes.

@delatbabel
Copy link
Author

I added a gateway interface for bulksms.com plus test case, and updated some documentation. For some strange reason I can't get the unit tests to run on my local development environment, it's PHP 5.5.9 with phpUnit 3.7.28 and gives me an error as follows:

PHP Fatal error: Class 'Buzz\Client\AbstractStream' not found in /home/del/develop/github/xi-sms/vendor/kriswallsmith/buzz/test/Buzz/Test/Client/AbstractStreamTest.php on line 11

Clearly some dependency issue that phpUnit is not picking up from the vendor/ tree when it should be. Let me know if you have any success with the unit tests.

@pekkis
Copy link
Member

pekkis commented Jan 8, 2015

Thanks! Will go thru and comment further!

@delatbabel
Copy link
Author

I just added a few more small documentation changes.

protected $bits;

/**
* Transient error codes.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Project uses 4 space indentation.

@delatbabel
Copy link
Author

I converted to 4 space indents.

if ($stop_dup_id > 0) {
$post_fields['stop_dup_id'] = $this->make_stop_dup_id();
}
$post_body = '';
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would the following work?

return http_build_query($post_fields);

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, not quite, because of the way that http_build_query encodes spaces.

@PKJedi
Copy link
Member

PKJedi commented Feb 8, 2015

Sorry for the delay. The documentation stuff is looks good, could move that into a separate PR? The gateway implementation has some further CS issues, try and make it match PSR rules. (camelCase, tabs->4spaces, etc.)

if( mb_detect_encoding($body, 'UTF-8') != 'UTF-8' ) {
$body = utf8_encode($body);
}
for ( $i = 0; $i < mb_strlen( $body, 'UTF-8' ); $i++ ) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would the following work?

$from = array_keys($special_chrs);
$to = array_values($special_chrs);
$ret_msg = str_replace($from, $to, $body);

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would for 7 bit messages but not for 16 bit messages because it would not be unicode safe.

@delatbabel
Copy link
Author

It's not really feasible for me to roll back a month old PR to start again and re-work it into two separate PRs. I can work on fixing the gateway code later but it was based on the implementation supplied to me from Bulk SMS.

To be honest I thought this project had been abandoned so I started working on something else.

@delatbabel
Copy link
Author

Unit tests are working fine now except for a long standing error with the MessageBirdGatewayTest. This uses constants that aren't declared, and should be using a mock but is not. I will PR that later.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants