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

Support PSR-4 #1779

Closed
samdark opened this issue Jan 4, 2014 · 28 comments
Closed

Support PSR-4 #1779

samdark opened this issue Jan 4, 2014 · 28 comments
Assignees
Milestone

Comments

@samdark
Copy link
Member

@samdark samdark commented Jan 4, 2014

PSR-4 was implemented in Composer (pre-release branch as far as I can tell) so it worth considering re-arranging our code structure to be a bit more simpler.

http://seld.be/notes/psr-4-autoloading-support-in-composer

@yiisoft/core-developers thoughts?

@qiangxue
Copy link
Member

@qiangxue qiangxue commented Jan 4, 2014

@cebe and I talked about this. In general, we should NOT move the directories. Otherwise we will "lose" the history of the moved code. We may adjust our composer.json files though to use PSR-4. We also need to modify our composer extension to support PSR-4.

@samdark
Copy link
Member Author

@samdark samdark commented Jan 4, 2014

What's the benefit for us then?

@qiangxue
Copy link
Member

@qiangxue qiangxue commented Jan 4, 2014

not much benefit. :)

@samdark
Copy link
Member Author

@samdark samdark commented Jan 4, 2014

I don't think history will be lost if there are no edits to the file and just directory re-arrangements in a single commit and then namespace corrections in a second commit.

@cebe
Copy link
Member

@cebe cebe commented Jan 4, 2014

History in subsplit repos gets lost when we move files from directories that are not part of the subsplit after that. I think we can wait with this until RC as there is no real benefit.

@samdark
Copy link
Member Author

@samdark samdark commented Jan 4, 2014

The more we wait the more history will be lost :) The benefit is that framework will look a bit less complicated for ones who'll dig into vendors. Also repo structure should be a bit simpler.

@samdark
Copy link
Member Author

@samdark samdark commented Jan 4, 2014

But overall I agree that it's not critical to actually move stuff, just an enhancement.

@cebe
Copy link
Member

@cebe cebe commented Jan 4, 2014

At least we have to wait for composer to officially release the feature and then a bit to let all people update.

@samdark
Copy link
Member Author

@samdark samdark commented Jan 4, 2014

PSR members are doing it in separate branches. With dev build of composer it already works so can be tested.

@zergussino
Copy link

@zergussino zergussino commented Jan 5, 2014

Not sure if this is proper here but :
http://philsturgeon.co.uk/blog/2014/01/composer-now-supports-psr4

... Jordi wrote a blog about why you don't want to switch all of your packages immediately to PSR-4 as many users won't be reminded to update for another 30 days, so for now I am suggesting people create a feature branch called "feature/psr4" for those who want to try it out...

@samdark
Copy link
Member Author

@samdark samdark commented Jan 6, 2014

Composer just updated with PSR-4 support: https://github.com/composer/composer/releases/tag/1.0.0-alpha8

@cebe
Copy link
Member

@cebe cebe commented Jan 9, 2014

Is this fixed by e123428 ?

@cebe
Copy link
Member

@cebe cebe commented Jan 9, 2014

Oh okay, we might reconsider moving directories...

@qiangxue
Copy link
Member

@qiangxue qiangxue commented Jan 9, 2014

@cebe: my change is about Yii autoloader. I dropped the support for PEAR-styled class names, because right now we use both composer autoloader and Yii autoloader. The Yii autoloader can be stricter because the the composer one will handle PEAR-styled classes.

@samdark
Copy link
Member Author

@samdark samdark commented Jan 9, 2014

That's definitely a good change. The question about eliminating extra directories is still open.

@qiangxue
Copy link
Member

@qiangxue qiangxue commented Jan 9, 2014

What are the benefits of removing the yii layer from the directories?

@samdark
Copy link
Member Author

@samdark samdark commented Jan 9, 2014

Only one benefit: less directories so it looks a tiny bit less complex.

@qiangxue
Copy link
Member

@qiangxue qiangxue commented Jan 9, 2014

The biggest drawback of the moving is that it will "lose" the history. For example, https://github.com/yiisoft/yii2-gii/commits/master/CodeFile.php shows only one record, although in PhpStorm I can see the whole history.

Also, when will composer 1.0 GA be released?

I suggest we have a vote about this. @yiisoft/core-developers could you cast your vote about this?

@cebe
Copy link
Member

@cebe cebe commented Jan 9, 2014

@qiangxue I'd like to try some git magic before moving and check whether we can keep history even when moving. In general I am for removing the yii dir.

@ghost ghost assigned cebe Jan 9, 2014
@samdark
Copy link
Member Author

@samdark samdark commented Jan 9, 2014

I'm for removing the dir as well.

btw., tried psr-4 for an extension. Works well.

@mdomba
Copy link
Member

@mdomba mdomba commented Jan 10, 2014

I'm for removing too

@creocoder
Copy link
Contributor

@creocoder creocoder commented Jan 10, 2014

I'm not core dev, but i'm too for removing, currently at alpha stage history is not so important.

@qiangxue
Copy link
Member

@qiangxue qiangxue commented Jan 10, 2014

Should we do it now? Note that composer 1.0 is not GA yet. Will this affect our release schedule?

@samdark
Copy link
Member Author

@samdark samdark commented Jan 10, 2014

No, it will not. Composer is already distributed with PSR-4 support and self-update command updates it to have PSR-4 support as well.

@cebe
Copy link
Member

@cebe cebe commented Jan 10, 2014

I'd like to check the history thing in git first. Will do this the next days.

@cebe
Copy link
Member

@cebe cebe commented Jan 11, 2014

@cebe cebe closed this Jan 11, 2014
@samdark
Copy link
Member Author

@samdark samdark commented Jan 12, 2014

Shouldn't extensions/composer/Installer.php be adjusted as well?

@qiangxue
Copy link
Member

@qiangxue qiangxue commented Jan 12, 2014

Already adjusted

On Sunday, January 12, 2014, Alexander Makarov wrote:

Shouldn't extensions/composer/Installer.php be adjusted as well?


Reply to this email directly or view it on GitHubhttps://github.com//issues/1779#issuecomment-32123901
.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
6 participants
You can’t perform that action at this time.