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

Rename Configuration.php to ApiClient.php #2

Closed
wants to merge 1 commit into from

Conversation

katosan
Copy link

@katosan katosan commented Aug 12, 2020

composer autoload can't find the class otherwise

composer autoload can't find the class otherwise
@mc2
Copy link

mc2 commented Aug 13, 2020

Does something need to be done to specify a stable release for Composer?

$ composer require mailchimp/marketing 
                                                                               
  [InvalidArgumentException]                                                   
  Could not find package mailchimp/marketing at any version for your minimum-  
  stability (stable). Check the package spelling or your minimum-stability

@alexis78i
Copy link

@mc2
I set these two lines in my composer.json

 "minimum-stability": "dev",
  "prefer-stable" : true,

like this, and it works.

{
  "minimum-stability": "dev",
  "prefer-stable" : true,
  "require": {
    "mailchimp/transactional": "*",
    "mailchimp/marketing": "dev-master",
  }
}

@mj4444ru
Copy link

Does something need to be done to specify a stable release for Composer?

$ composer require mailchimp/marketing:dev-master

@mc2
Copy link

mc2 commented Aug 14, 2020

thanks all!
I thought I saw an email address for support with this API. If you have it, please let me know.

@katosan
Copy link
Author

katosan commented Aug 14, 2020

Does something need to be done to specify a stable release for Composer?

$ composer require mailchimp/marketing:dev-master

If you want a stable version you have to tag a commit sematically e.g. 1.0.0 Composer will then determin the latest possible version for your stability requirements. e.g. composer require mailchimp/marketing:1.1.1 will not match 1.0.0

@AreSanito
Copy link

Hello, I pulled your branch to try to fix my bug, but I still got trouble, I'm using CodeIgniter framework with composer autoloader.
I'm having this issue : Message: Class 'MailchimpMarketing\ApiClient' not found
What should I change ?

use MailchimpMarketing\ApiClient;

class Auth extends CI_Controller
{
	public function test()
	{

		$mailchimp = new MailchimpMarketing\ApiClient();

		$mailchimp->setConfig([
			'apiKey' => 'xxxxxxxxxxxxxxxxxxxxxx-us17',
			'server' => 'us17'
		]);

		$response = $mailchimp->ping->get();
		print_r($response);
	}
}

Thanks in advance

@martinbean
Copy link

Any update on this? I can’t use this package because the filename doesn’t match the class name.

It’s also disappointing there seems to be no tagging going on this repository. That would be tremendously helpful when installing this package via Composer, too.

@riconeitzel
Copy link

Any update on this? I can’t use this package because the filename doesn’t match the class name.

You could use https://github.com/cweagans/composer-patches in the meantime. That’s quite useful for situations like this.

Rico

@martinbean
Copy link

You could use https://github.com/cweagans/composer-patches in the meantime. That’s quite useful for situations like this.

I’ve just forked the package and made the one-line change myself.

I wasn’t impeded by installing the library through other methods, but just curious as to why such a trivial change has been left unmerged for nearly a month now?

@philcook
Copy link

philcook commented Sep 7, 2020

There is a line in the composer.json file that should do a copy of the Configuration.php file to ApiClient.php however this doesn't appear to be working for a lot of people.
I suggest this needs to be fixed rather than just make a copy and put in the repo.

The original developer probably did it this way for a reason.

    "scripts": {
        "pre-autoload-dump": "mv ./lib/Configuration.php ./lib/ApiClient.php"
    },

@riconeitzel
Copy link

The original developer probably did it this way for a reason.

    "scripts": {
        "pre-autoload-dump": "mv ./lib/Configuration.php ./lib/ApiClient.php"
    },

that makes no sense at all to me 😄 but good find!

@philcook
Copy link

philcook commented Sep 7, 2020

The original developer probably did it this way for a reason.

    "scripts": {
        "pre-autoload-dump": "mv ./lib/Configuration.php ./lib/ApiClient.php"
    },

that makes no sense at all to me but good find!

Yes it doesn't make sense to me either but a thought maybe that there is a central codebase for their API code that in turn churns it out to all the programming languages including PHP, Ruby etc.

I guess we will have to await for the original developer to do something here.

@philcook
Copy link

philcook commented Sep 7, 2020

Just worked out as a sticky plaster fix for everyone here if you add the following to your projects composer.json file this will work.

"scripts": { "pre-autoload-dump": "cp ./vendor/mailchimp/marketing/lib/Configuration.php ./vendor/mailchimp/marketing/lib/ApiClient.php" },

This fix will also work when I commit for other developers and our automated deployment system as its reading from the composer file.

@dadaxr
Copy link

dadaxr commented Sep 8, 2020

The original developer probably did it this way for a reason.

    "scripts": {
        "pre-autoload-dump": "mv ./lib/Configuration.php ./lib/ApiClient.php"
    },

that makes no sense at all to me but good find!

Yes it doesn't make sense to me either but a thought maybe that there is a central codebase for their API code that in turn churns it out to all the programming languages including PHP, Ruby etc.

I guess we will have to await for the original developer to do something here.

As mentioned in the composer doc :

What is a script?#

A script, in Composer's terms, can either be a PHP callback (defined as a static method) or any command-line executable command. Scripts are useful for executing a package's custom code or package-specific commands during the Composer execution process.

Note: **Only scripts defined in the root package's composer.json are executed. If a dependency of the root package specifies its own scripts, Composer does not execute those additional scripts.

IMHO, renaming the file to ApiClient.php to stick to the PSR-4 standard is the right thing to do, but if for any reason they must keep this name, composer has a special autoload option : classmap which allow file name and class name to be different.

I you want people to use your official library, please make it work ;)

@martinbean
Copy link

[If] you want people to use your official library, please make it work ;)

💯

@miradesne
Copy link

Can we get this merged if possible?:)

@grasmachien
Copy link

grasmachien commented Sep 23, 2020

After changing the filename I was greeted with another error.

Parse error: syntax error, unexpected 'list' (T_LIST), expecting identifier (T_STRING) in /vendor/mailchimp/marketing/lib/Api/AuthorizedAppsApi.php on line 65

I'm not a full time PHP developer but it seems that "list" is a reserved word and should not be used as a function...
So, I'm giving up on this library.

@riconeitzel
Copy link

After changing the filename I was greeting with another error.

Parse error: syntax error, unexpected 'list' (T_LIST), expecting identifier (T_STRING) in /vendor/mailchimp/marketing/lib/Api/AuthorizedAppsApi.php on line 65

You broke something in AuthorizedAppsApi.php in Line 64 ... that’s not caused by the rename.

@grasmachien
Copy link

You broke something in AuthorizedAppsApi.php in Line 64 ... that’s not caused by the rename.

Really weird that it is my fault because I did not change anything besides the filename :)
Also, I'm not saying the issue is caused by changing the filename.
What is the required minimum PHP version for this lib?
I can't find it in the readme.

@riconeitzel
Copy link

I’d guess at least php7 - we’re using it with 7.2 and 7.4

@jonwinstanley
Copy link

I have tweeted the maintainer of this project and asked what is supposed to happen with this library. Currently the getting started code does not work.

@martinbean
Copy link

Come on. Are you telling me a company with the budget and number of engineers that Mailchimp has, can’t spend an hour fixing a library for using their product? 😩

@cheong999
Copy link

cheong999 commented Sep 28, 2020

After changing the filename I was greeted with another error.

Parse error: syntax error, unexpected 'list' (T_LIST), expecting identifier (T_STRING) in /vendor/mailchimp/marketing/lib/Api/AuthorizedAppsApi.php on line 65

I'm not a full time PHP developer but it seems that "list" is a reserved word and should not be used as a function...
So, I'm giving up on this library.

I am running PHP 5.6, and I am facing this error too. With this error, this is unusable. Will be great if the MailChimp could rectify it as soon as possible.

based on https://www.php.net/manual/en/reserved.keywords.php, list() is a reserved keyword

@Moishe
Copy link
Contributor

Moishe commented Sep 29, 2020

Hey all, sorry for the delays here; the new version pushed on Friday should address this issue (I just tested it on my machine from a clean install). Let me know if you see any more problems and, again, apologies for the delays.

@Moishe
Copy link
Contributor

Moishe commented Sep 29, 2020

One other thing: this SDKs (and the SDKs for other languages) are generated from a codegen repo here: https://github.com/mailchimp/mailchimp-client-lib-codegen

We won't accept PRs on this repo (though we will use them to inform bug fixes) because they'll be overridden by the automated codegen. If you have improvements to the SDKs, the codegen repo is the place for PRs.

Thanks again for your patience / apologies it took so long.

@akutaktau
Copy link

composer require mailchimp/marketing:dev-master

i'm using this to install the library.

@dregad
Copy link

dregad commented Oct 15, 2020

Does something need to be done to specify a stable release for Composer?

@mc2 This problem has been fixed in the latest release (3.0.22): the version is now properly tagged in the repo, allowing Packagist/Composer to pick it up.

So running composer require mailchimp/marketing will now set composer.json as expected

    "require": {
        "mailchimp/marketing": "^3.0"
    }

and there is no need to mess with @dev suffix, requiring dev-master or relying on "minimum-stability": "dev" anymore 👍

As for this issue's original problem, it has also been fixed a couple weeks ago as mentioned by @Moishe, so I think this discussion can be closed.

@martinbean
Copy link

Just installed mailchimp/marketing=^3.0 in a project and it does indeed seem to be fixed 🎉

@Moishe
Copy link
Contributor

Moishe commented Oct 15, 2020

Gonna close this, because it should be resolved now. In the future please make PRs on the mailchimp-client-codegen repo (and we'll try to respond to them more quickly)

@Moishe Moishe closed this Oct 15, 2020
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.