Hello, and thank you for this module. I've been looking into this module as I'm considering using its services in a new module I'm developing. However, I found some issues with it, so I'll post a couple of tickets.

According to the Drupal CVS FAQ, Drupal should not host "... GPL code/images/files that are available from 3rd party sites as long as the proper version to use with your module is easily accessible". I definitely see why this is tempting, but it's still not a good practice.

Comments

ilo’s picture

Hi Kirie.

PHP5 SOAP extension and nuSOAP uses the same names for soap class declaration, and obviously if PHP5 SOAP extension is enabled, you can't include/require the nusoap files at all. The included version in this repository is a modified class of the nuSOAP sources, so it can co-exists with PHP5 SOAP extension enabled.

I'm not sure if search/download/modify could be considered easily accesible, but I'm not friend of including third parties in the repository also. From the user pov, nuSOAP is required for the module to work if PHP5 SOAP extension is not enabled, or it will not be usable.

Users will expect to "install and use" the module, so to remove the nuSOAP I should have to address the wysiwyg style, and set clear instructions on how to download nuSOAP, and modify the files.

I don't know what to do, need some advice.

ilo’s picture

Status: Active » Needs work

Actually, I've seen the nusoap version in the repository is 0.7.2, and last version is 0.7.3. This last version includes a protection in case of PHP5 SOAP extension is enabled, so.. as it is time to upgrade nuSOAP version, and it includes a protection for PHP loaded extension, I guess it will not require any change in the files, so it wont neither be required to have in the CVS.

If nobody complains, the nuSOAP code will be removed form the module folder. For this some checks should be included, a configuration option should be provided to locate the nuSOAP folder within the filesystem, and should be documented.

There are other files that should be reviewed (AES128.php) as well.

ilo’s picture

I've done some changes to soapclient and now Nusoap library no longer needs to be in the CVS, as the module should work with nusoap version available in SourceForge. As the location of the library is now configurable: #581202: Make nuSOAP library location configurable, I feel confident to remove the old library code from the CVS.

ilo’s picture

Status: Needs work » Fixed

I've included the download information in the project page, and removed the CVS of the 6.x-1.x-dev branch.
Commited to 6.x-1.x-dev

Status: Fixed » Closed (fixed)

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