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

make file structure PSR-0 compatible #2

Open
wants to merge 7 commits into
base: psr0
Choose a base branch
from

Conversation

caseycs
Copy link
Contributor

@caseycs caseycs commented Jan 25, 2013

move global constants to class, add gitignore

@romainneutron
Copy link
Owner

Great !

I think it's great to be PSR-0/1/2 compliant. One thing is disagree on this changes is the lack of support for PHP 5.2. I think we should not use namespaces, and probably move functions as methods of SphinxClient object.

Now, let's ping SphinxSearch developers and see how can it be maintained in the upcoming 2.0.7 versions and integrate SphinxSearch sources to ease the update of this repo.

@caseycs
Copy link
Contributor Author

caseycs commented Jan 25, 2013

Good notices. I'll remove namespace. About functions - i agree, i'll move them.

@romainneutron
Copy link
Owner

For the record, I opened a thread on SphinxSearch forum : http://sphinxsearch.com/forum/view.html?id=10618

@atrauzzi
Copy link

Wondering when I can expect to see these changes in the master branch. The constants issue is a bit of a blocker for anyone trying to use this library properly.

@romainneutron
Copy link
Owner

This is not blocking as composer autoloads sphinxapi.php correctly. Using this PSR0 modified version would lead to another more blocking issue : re-do the job for every new Sphinx release.

As long as SphinxSearch php developers themselves do not use PSR-0 and avoid constants use, I'd rather prefer not to modify their jobs.

A workaround could be to create a branch in this project with tagged psr-0 versions of sphinx versions. Would it be a valid solution for you guys ?

@caseycs
Copy link
Contributor Author

caseycs commented May 29, 2013

As for me - it will be great. We also can update readme.md, see commit below.

@atrauzzi
Copy link

The problem is that you can't resolve the constants without first triggering a load of the class. I'm just asking that commit 127c5ab be merged in and made available now as the current version that composer is pointing to is still using defines.

@KarelWintersky
Copy link

2019 year.
PHP 5.2 not supported more than 5 years ago.

I think, we CAN MERGE this PR now.

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.

5 participants