PSR-0 compliance #42

Closed
wants to merge 3 commits into
from

Conversation

Projects
None yet
@BenMorel

Most PHP libraries are now complying with the PSR-0 standard for autoloader interoperability.
Here is the simplest modification to comply with this standard.

This makes PHP 5.3 the minimum required version; as the current version is 5.4.8, it may be time for this change.

Examples & tests are running ok.

The PSR-0 standard: https://github.com/php-fig/fig-standards/blob/master/accepted/PSR-0.md

@daaku

This comment has been minimized.

Show comment Hide comment
@daaku

daaku Oct 30, 2012

Contributor

We're not ready for a backward incompatible change, specifically the use of namespaces at a quick glance. It would be great if you could split the change to only apply backward compatible fixes for PSR-0 compliance. Thanks for the patch!

Contributor

daaku commented Oct 30, 2012

We're not ready for a backward incompatible change, specifically the use of namespaces at a quick glance. It would be great if you could split the change to only apply backward compatible fixes for PSR-0 compliance. Thanks for the patch!

@BenMorel

This comment has been minimized.

Show comment Hide comment
@BenMorel

BenMorel Oct 31, 2012

I removed the namespaces as per your request, but then this is not PSR-0 anymore (every class must belong to a vendor namespace). We're still closer than the original implementation, though.

I'm wondering, why not creating a new major version (3.3.0 or even 4.0.0) introducing namespaces, fully PSR-0 compatible, PHP 5.3, while keeping the 3.2.x branch for PHP 5.2, that you would deprecate and decide of an end-of-life date (like 6 months from now)?

You'd just have to maintain two branches for a few months, but given the number of recent commits, that shouldn't be a pain?

Also, the move to PHP 5.3 is now unavoidable: even the snail Red Hat Enterprise Linux has now moved to this version.
If we consider that most of Facebook SDK users are already using PHP 5.3, the changes to their applications will be very small to start using the new library.

What do you think?

I removed the namespaces as per your request, but then this is not PSR-0 anymore (every class must belong to a vendor namespace). We're still closer than the original implementation, though.

I'm wondering, why not creating a new major version (3.3.0 or even 4.0.0) introducing namespaces, fully PSR-0 compatible, PHP 5.3, while keeping the 3.2.x branch for PHP 5.2, that you would deprecate and decide of an end-of-life date (like 6 months from now)?

You'd just have to maintain two branches for a few months, but given the number of recent commits, that shouldn't be a pain?

Also, the move to PHP 5.3 is now unavoidable: even the snail Red Hat Enterprise Linux has now moved to this version.
If we consider that most of Facebook SDK users are already using PHP 5.3, the changes to their applications will be very small to start using the new library.

What do you think?

@conradkdotcom

This comment has been minimized.

Show comment Hide comment
@conradkdotcom

conradkdotcom Oct 31, 2012

1+ on the new major version idea. Namespaces would be very helpful.

1+ on the new major version idea. Namespaces would be very helpful.

@ikocev

This comment has been minimized.

Show comment Hide comment
@ikocev

ikocev Oct 31, 2012

yep, new major version is good idea. (+1)

ikocev commented Oct 31, 2012

yep, new major version is good idea. (+1)

@eduardbulai

This comment has been minimized.

Show comment Hide comment
@eduardbulai

eduardbulai Oct 31, 2012

Absolutely pro for new major version. (+1)

Absolutely pro for new major version. (+1)

@ricardclau

This comment has been minimized.

Show comment Hide comment
@ricardclau

ricardclau Oct 31, 2012

Keeping compatibility with PHP5.2 is an error to my point of view.

PHP5.2 has been abandoned for more than 1 year. Everyone who uses projects that are constantly updated are either on 5.3 or they are not likely to downloaded an updated version of the API that could break something even if you promise backwards compatibility.

Another major problem is the lack of possibility to extend static timeout properties (because of Late Static Bindings). 60 seconds? WTF!

Absolutely +1 for PHP5.3, namespaces, etc... and leave current version as a tag for 5.2 users.

Keeping compatibility with PHP5.2 is an error to my point of view.

PHP5.2 has been abandoned for more than 1 year. Everyone who uses projects that are constantly updated are either on 5.3 or they are not likely to downloaded an updated version of the API that could break something even if you promise backwards compatibility.

Another major problem is the lack of possibility to extend static timeout properties (because of Late Static Bindings). 60 seconds? WTF!

Absolutely +1 for PHP5.3, namespaces, etc... and leave current version as a tag for 5.2 users.

@daaku

This comment has been minimized.

Show comment Hide comment
@daaku

daaku Oct 31, 2012

Contributor

Thanks for the feedback all, I'll talk to some folks internally and get back.

Contributor

daaku commented Oct 31, 2012

Thanks for the feedback all, I'll talk to some folks internally and get back.

@yahuarkuntur

This comment has been minimized.

Show comment Hide comment
@yahuarkuntur

yahuarkuntur Nov 1, 2012

👍

👍

@richardpickett

This comment has been minimized.

Show comment Hide comment
@richardpickett

richardpickett Nov 1, 2012

How do we start a whole new discussion? I don't want to hijack this thread,
but I'd like to contribute by expanding the example.php with an example of
each type of activity (wall post, video post, image post, comment, etc...)
and am looking for a concise list of parameters for each of those
"functions" to build a comprehensive example of each (with optional and
mandatory parameters listed).

Anyone that knows how to start a new thread without doing a commit or has
something to help me put this list together, I'd appreciate it.

You can contact me directly: (I have excellent spam control)
Richard.Pickett@CSRTechnologies.com

Thanks, be blessed,

Richard W. Pickett, Jr.

On Thu, Nov 1, 2012 at 11:52 AM, Brian Debuire notifications@github.comwrote:

[image: 👍]


Reply to this email directly or view it on GitHubhttps://github.com/facebook/facebook-php-sdk/pull/42#issuecomment-9987142.

How do we start a whole new discussion? I don't want to hijack this thread,
but I'd like to contribute by expanding the example.php with an example of
each type of activity (wall post, video post, image post, comment, etc...)
and am looking for a concise list of parameters for each of those
"functions" to build a comprehensive example of each (with optional and
mandatory parameters listed).

Anyone that knows how to start a new thread without doing a commit or has
something to help me put this list together, I'd appreciate it.

You can contact me directly: (I have excellent spam control)
Richard.Pickett@CSRTechnologies.com

Thanks, be blessed,

Richard W. Pickett, Jr.

On Thu, Nov 1, 2012 at 11:52 AM, Brian Debuire notifications@github.comwrote:

[image: 👍]


Reply to this email directly or view it on GitHubhttps://github.com/facebook/facebook-php-sdk/pull/42#issuecomment-9987142.

@BenMorel

This comment has been minimized.

Show comment Hide comment
@BenMorel

BenMorel Nov 3, 2012

Thanks daaku. If that may help your decision, for information Amazon Web Services have released today their SDK for PHP 2, which is PHP 5.3, PSR-0, PSR-1 and PSR-2 compliant.

BenMorel commented Nov 3, 2012

Thanks daaku. If that may help your decision, for information Amazon Web Services have released today their SDK for PHP 2, which is PHP 5.3, PSR-0, PSR-1 and PSR-2 compliant.

@vpassapera

This comment has been minimized.

Show comment Hide comment
@vpassapera

vpassapera Jan 2, 2013

Is a decision to make Facebook's code PSR 0 1 and 2 compliant in the horizon?

Is a decision to make Facebook's code PSR 0 1 and 2 compliant in the horizon?

matthew-muscat added a commit to matthew-muscat/facebook-php-sdk that referenced this pull request Feb 8, 2013

Converted classes to namespaces, PSR-0 compliant
Converted classes to namespaces, PSR-0 compliant as per @BenMorel patch (pull request #42)

Signed-off-by: Matthew Muscat <matthew@mamis.com.au>
@jasonrhodes

This comment has been minimized.

Show comment Hide comment
@jasonrhodes

jasonrhodes Feb 9, 2013

Another vote for accepting this PR with the namespaces and moving to 5.3 support. Big giants like Amazon and Facebook can help move everybody forward (5.5 is on the horizon now...)

Another vote for accepting this PR with the namespaces and moving to 5.3 support. Big giants like Amazon and Facebook can help move everybody forward (5.5 is on the horizon now...)

@philsturgeon

This comment has been minimized.

Show comment Hide comment
@philsturgeon

philsturgeon Feb 19, 2013

Contributor

How has this still not been done, started, or accepted yet? PHP 5.2 usage numbers are way down, other than the huge number of WordPress installs keeping the numbers bumped up.

Maintain a separate branch with only security fixes, and the new major version is PHP 5.3 only.

Contributor

philsturgeon commented Feb 19, 2013

How has this still not been done, started, or accepted yet? PHP 5.2 usage numbers are way down, other than the huge number of WordPress installs keeping the numbers bumped up.

Maintain a separate branch with only security fixes, and the new major version is PHP 5.3 only.

@MichMich

This comment has been minimized.

Show comment Hide comment
@MichMich

MichMich Feb 19, 2013

+1 for this one. 
Vriendelijke groeten,
Michael Teeuw


Sent from my iPhone.

Michael Teeuw
Xonay Media
www.xonaymedia.nl
06 51 71 36 15

On Tue, Feb 19, 2013 at 11:27 PM, Phil Sturgeon notifications@github.com
wrote:

How has this still not been done, started, or accepted yet? PHP 5.2 usage numbers are way down, other than the huge number of WordPress installs keeping the numbers bumped up.

Maintain a separate branch with only security fixes, and the new major version is PHP 5.3 only.

Reply to this email directly or view it on GitHub:
facebook#42 (comment)

+1 for this one. 
Vriendelijke groeten,
Michael Teeuw


Sent from my iPhone.

Michael Teeuw
Xonay Media
www.xonaymedia.nl
06 51 71 36 15

On Tue, Feb 19, 2013 at 11:27 PM, Phil Sturgeon notifications@github.com
wrote:

How has this still not been done, started, or accepted yet? PHP 5.2 usage numbers are way down, other than the huge number of WordPress installs keeping the numbers bumped up.

Maintain a separate branch with only security fixes, and the new major version is PHP 5.3 only.

Reply to this email directly or view it on GitHub:
facebook#42 (comment)

@robertofrega

This comment has been minimized.

Show comment Hide comment
@robertofrega

robertofrega Feb 19, 2013

Me too. +1 on this matter.
Em 19/02/2013 19:29, "MichMich" notifications@github.com escreveu:

+1 for this one.
Vriendelijke groeten,
Michael Teeuw


Sent from my iPhone.

Michael Teeuw
Xonay Media
www.xonaymedia.nl
06 51 71 36 15

On Tue, Feb 19, 2013 at 11:27 PM, Phil Sturgeon notifications@github.com

wrote:

How has this still not been done, started, or accepted yet? PHP 5.2
usage numbers are way down, other than the huge number of WordPress
installs keeping the numbers bumped up.
Maintain a separate branch with only security fixes, and the new major

version is PHP 5.3 only.

Reply to this email directly or view it on GitHub:

facebook#42 (comment)


Reply to this email directly or view it on GitHubhttps://github.com/facebook/facebook-php-sdk/pull/42#issuecomment-13804046.

Me too. +1 on this matter.
Em 19/02/2013 19:29, "MichMich" notifications@github.com escreveu:

+1 for this one.
Vriendelijke groeten,
Michael Teeuw


Sent from my iPhone.

Michael Teeuw
Xonay Media
www.xonaymedia.nl
06 51 71 36 15

On Tue, Feb 19, 2013 at 11:27 PM, Phil Sturgeon notifications@github.com

wrote:

How has this still not been done, started, or accepted yet? PHP 5.2
usage numbers are way down, other than the huge number of WordPress
installs keeping the numbers bumped up.
Maintain a separate branch with only security fixes, and the new major

version is PHP 5.3 only.

Reply to this email directly or view it on GitHub:

facebook#42 (comment)


Reply to this email directly or view it on GitHubhttps://github.com/facebook/facebook-php-sdk/pull/42#issuecomment-13804046.

@zackkitzmiller

This comment has been minimized.

Show comment Hide comment
@zackkitzmiller

zackkitzmiller Feb 19, 2013

+1

@floristenhove

This comment has been minimized.

Show comment Hide comment
@floristenhove

floristenhove Feb 19, 2013

Yes please, +1!

Regards,

Floris

Op 19 feb. 2013 om 23:35 heeft Zack Kitzmiller notifications@github.com
het volgende geschreven:

+1


Reply to this email directly or view it on
GitHubhttps://github.com/facebook/facebook-php-sdk/pull/42#issuecomment-13804254.

Yes please, +1!

Regards,

Floris

Op 19 feb. 2013 om 23:35 heeft Zack Kitzmiller notifications@github.com
het volgende geschreven:

+1


Reply to this email directly or view it on
GitHubhttps://github.com/facebook/facebook-php-sdk/pull/42#issuecomment-13804254.

This comment has been minimized.

Show comment Hide comment
@ghost

ghost Feb 19, 2013

As soon as possible.

ghost commented Feb 19, 2013

As soon as possible.

@hasokeric

This comment has been minimized.

Show comment Hide comment
@hasokeric

hasokeric Feb 19, 2013

+1

+1

@conradkdotcom

This comment has been minimized.

Show comment Hide comment
@conradkdotcom

conradkdotcom Feb 19, 2013

+1

@ricardclau

This comment has been minimized.

Show comment Hide comment
@ricardclau

ricardclau Feb 19, 2013

With composer support we also had to wait for quite a while.

Come on guys, you are Facebook, you've pushed PHP and MySQL farther than most people would have expected, I honestly cannot understand why this is taking so long.

People on 5.2 in their servers don't care about updating, and they can always get some old branch/tag.

Please, let's move forward, only companies like yours can make the whole community advance.

With composer support we also had to wait for quite a while.

Come on guys, you are Facebook, you've pushed PHP and MySQL farther than most people would have expected, I honestly cannot understand why this is taking so long.

People on 5.2 in their servers don't care about updating, and they can always get some old branch/tag.

Please, let's move forward, only companies like yours can make the whole community advance.

@gbuckingham89

This comment has been minimized.

Show comment Hide comment
@gbuckingham89

gbuckingham89 Feb 19, 2013

+1

@manuelhe

This comment has been minimized.

Show comment Hide comment
@manuelhe

manuelhe Feb 20, 2013

+1

El martes, 19 de febrero de 2013, George Buckingham escribió:

+1


Reply to this email directly or view it on GitHubhttps://github.com/facebook/facebook-php-sdk/pull/42#issuecomment-13806826.

/* manuel.he - @fractalsoftware - http://linkd.in/qVvGrL */

+1

El martes, 19 de febrero de 2013, George Buckingham escribió:

+1


Reply to this email directly or view it on GitHubhttps://github.com/facebook/facebook-php-sdk/pull/42#issuecomment-13806826.

/* manuel.he - @fractalsoftware - http://linkd.in/qVvGrL */

@matthew-muscat

This comment has been minimized.

Show comment Hide comment
@matthew-muscat

matthew-muscat Feb 20, 2013

+1 here

+1 here

@ipalaus

This comment has been minimized.

Show comment Hide comment
@ipalaus

ipalaus Feb 20, 2013

+1

ipalaus commented Feb 20, 2013

+1

@blitux

This comment has been minimized.

Show comment Hide comment
@blitux

blitux Feb 20, 2013

+1 on new branch for security fixes only and new major version with PSR compliance.

blitux commented Feb 20, 2013

+1 on new branch for security fixes only and new major version with PSR compliance.

@SlKelevro

This comment has been minimized.

Show comment Hide comment
@SlKelevro

SlKelevro Feb 20, 2013

definitely +1

definitely +1

@shanyee

This comment has been minimized.

Show comment Hide comment
@shanyee

shanyee Feb 20, 2013

+1

shanyee commented Feb 20, 2013

+1

@DisposaBoy

This comment has been minimized.

Show comment Hide comment
@DisposaBoy

DisposaBoy Feb 20, 2013

Quit it with +1, content-less spam already!

Quit it with +1, content-less spam already!

@maximebeaudoin

This comment has been minimized.

Show comment Hide comment
@maximebeaudoin

maximebeaudoin Feb 20, 2013

👍

👍

@stormpat

This comment has been minimized.

Show comment Hide comment
@stormpat

stormpat Mar 19, 2013

Agreed! This would be great!

Agreed! This would be great!

@raymondjavaxx

This comment has been minimized.

Show comment Hide comment
@raymondjavaxx

raymondjavaxx May 29, 2013

+1

@irazasyed

This comment has been minimized.

Show comment Hide comment
@irazasyed

irazasyed May 29, 2013

Contributor

Great idea, Totally agree!
But any news/updates? Been 7 months already.

Contributor

irazasyed commented May 29, 2013

Great idea, Totally agree!
But any news/updates? Been 7 months already.

@RobMasters

This comment has been minimized.

Show comment Hide comment
@RobMasters

RobMasters Jun 25, 2013

What's the hold up for this? I just downloaded this via Composer and couldn't believe that it wasn't properly namespaced. As it is, I simply can't use this library as there's already a Facebook class in the large, legacy application.

What's the hold up for this? I just downloaded this via Composer and couldn't believe that it wasn't properly namespaced. As it is, I simply can't use this library as there's already a Facebook class in the large, legacy application.

@thomaswelton

This comment has been minimized.

Show comment Hide comment
@thomaswelton

thomaswelton Jul 14, 2013

I have forked the commits @BenMorel had made to make this PSR-0 compliant and then merged the latest changes from the core Facebook repo up to v3.2.2 https://github.com/thomaswelton/facebook-php-sdk
This has also been submited to packagist https://packagist.org/packages/thomaswelton/facebook-php-sdk

This has allowed me to install it via composer and use proper namespaces for my Laravel package.

I have forked the commits @BenMorel had made to make this PSR-0 compliant and then merged the latest changes from the core Facebook repo up to v3.2.2 https://github.com/thomaswelton/facebook-php-sdk
This has also been submited to packagist https://packagist.org/packages/thomaswelton/facebook-php-sdk

This has allowed me to install it via composer and use proper namespaces for my Laravel package.

@BenMorel

This comment has been minimized.

Show comment Hide comment
@BenMorel

BenMorel Jul 14, 2013

@thomaswelton For information, you can still use the Facebook API with Composer even though it's not PSR-0, if you autoload your classes with require 'vendor/autoload.php';. Composer will have generated a class map to handle autoloading of the Facebook classes. But yep, I'd be happy to see Facebook namespaced!

@thomaswelton For information, you can still use the Facebook API with Composer even though it's not PSR-0, if you autoload your classes with require 'vendor/autoload.php';. Composer will have generated a class map to handle autoloading of the Facebook classes. But yep, I'd be happy to see Facebook namespaced!

@BenMorel

This comment has been minimized.

Show comment Hide comment
@BenMorel

BenMorel Jul 14, 2013

@daaku FYI, PHP 5.3 is reaching End Of Life, so the argument to support PHP 5.2 is getting really awkward!

@daaku FYI, PHP 5.3 is reaching End Of Life, so the argument to support PHP 5.2 is getting really awkward!

@thomaswelton

This comment has been minimized.

Show comment Hide comment
@thomaswelton

thomaswelton Jul 14, 2013

Yeah for anyone that just wants to install facebook via composer you can find the official package here https://packagist.org/packages/facebook/php-sdk

In the app I'm writing I needed the Facebook php-sdk to be properly namespaced. So for my own projects I'll be keeping my PSR-0 compliant package up to date.

Yeah for anyone that just wants to install facebook via composer you can find the official package here https://packagist.org/packages/facebook/php-sdk

In the app I'm writing I needed the Facebook php-sdk to be properly namespaced. So for my own projects I'll be keeping my PSR-0 compliant package up to date.

@BenMorel

This comment has been minimized.

Show comment Hide comment
@BenMorel

BenMorel Aug 3, 2013

This is being overlooked, I'm closing this PR.
I've opened a bug with my wishlist for the next major version instead:

https://developers.facebook.com/bugs/612436278789603

Please register your interest there if you want these changes as well!

BenMorel commented Aug 3, 2013

This is being overlooked, I'm closing this PR.
I've opened a bug with my wishlist for the next major version instead:

https://developers.facebook.com/bugs/612436278789603

Please register your interest there if you want these changes as well!

@BenMorel BenMorel closed this Aug 3, 2013

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