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

Real PSR-4 Autoloading #182

Merged
merged 1 commit into from
Jul 12, 2014
Merged

Real PSR-4 Autoloading #182

merged 1 commit into from
Jul 12, 2014

Conversation

GrahamCampbell
Copy link
Contributor

Replaces #181. TBH, there isn't really much merit in this, but it's here if you want it.

@GrahamCampbell
Copy link
Contributor Author

Any news on this?

@augustohp augustohp added this to the 0.7.0 milestone Jun 26, 2014
@augustohp
Copy link
Member

Sorry for the delay, although I really appreciate your effort, I miss the point of using PSR-4 over PSR-0?

I am inclined to merge you pull request based on your effort and contribution, but do we have any concrete benefit of using one instead of another?

@augustohp
Copy link
Member

Ping

@GrahamCampbell
Copy link
Contributor Author

Do you want me to rebase?

@GrahamCampbell
Copy link
Contributor Author

Ok, PSR-4 is the new standard over PSR-0, because it is more prcical and flexible. For example, it allows less directory nesting. When you install a package in composer that's PSR-0 autoloaded we end up with this sort of structure: "vendor\maintainer-name\package-name\src\MaintainerName\PackageName\Class.php". As you can see, that's quite long. A better system is PSR-4: ""vendor\maintainer-name\package-name\src\Class.php"". That smaller nesting is still useful when you are looking at the "src" folder in the repo itself too.

@GrahamCampbell
Copy link
Contributor Author

I've rebased and added a branch alias. They are a good practice.

@GrahamCampbell
Copy link
Contributor Author

Any news on this?

@augustohp
Copy link
Member

Any news on this?

Sorry! Can you separate the commit of the branch alias in a stand alone commit?

@augustohp
Copy link
Member

As you can see, that's quite long. A better system is PSR-4: ""vendor\maintainer-name\package-name\src\Class.php""

Agreed.

@GrahamCampbell
Copy link
Contributor Author

Sure. Give me 1 sec...

@GrahamCampbell
Copy link
Contributor Author

See #194. Also, refresh this page to see the new psr-4 commit. I force pushed it.

augustohp pushed a commit that referenced this pull request Jul 12, 2014
PSR-4 support instead of PSR-0.
@augustohp augustohp merged commit 95b6d31 into Respect:master Jul 12, 2014
@augustohp
Copy link
Member

Travis broke but I didn't check any further.

@GrahamCampbell
Copy link
Contributor Author

Could that be linked to the package.XML being wrong? I didn't update the paths in there. Is that file even update anyway?

alganet pushed a commit that referenced this pull request Jul 14, 2014
@alganet
Copy link
Member

alganet commented Jul 14, 2014

I've removed our custom autoloader from bootstrap.php and referenced only the composer autoload.php. This means that our tests depend on composer from now on. Tests are passing.

I'm not sure if we should keep the package.xml and PEAR dependencies. It seems that no one is using it.

@alganet
Copy link
Member

alganet commented Jul 14, 2014

Actually, hhvm is still failing. We need to figure out what to do with library/RecursiveTreeIterator.php, which hhvm does not implement. Under PSR-0 the autoload was seamless. Should we move the fallback implementation to our namespace and load under PSR-4?

@augustohp
Copy link
Member

I'm not sure if we should keep the package.xml and PEAR dependencies. It seems that no one is using it.

It is clearly dead.

@GrahamCampbell GrahamCampbell deleted the psr-4 branch July 14, 2014 14:21
@henriquemoody
Copy link
Member

Just a note:

I just saw that change because of some old pull requests of mine broken... PSR-0 and PSR-4 can work together, there was not need to rename the files but just update composer autoload.
Any way, that's past 😄

@GrahamCampbell
Copy link
Contributor Author

Just rebase your pulls.

@henriquemoody
Copy link
Member

@GrahamCampbell I already did it before comment here

@henriquemoody henriquemoody mentioned this pull request Jan 19, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants