Removed "Facebook" prefix form all classes #888

Nyholm · 2017-12-02T08:16:07Z

This will fix #609.

I've used PHPStorms "rename class" functionality to rename all the classes named Facebook*.

PS. Sorry fo an another massive PR.

Nyholm

Ive done a review and it all looks good

yguedidi · 2017-12-03T19:41:20Z

src/App.php


-class FacebookApp implements \Serializable
+class App implements \Serializable


as this PR is about renaming, what about naming this one Application?

yguedidi · 2017-12-03T19:46:24Z

src/PersistentData/MemoryPersistentDataHandler.php

 * @package Facebook
 */
-class FacebookMemoryPersistentDataHandler implements PersistentDataInterface
+class MemoryPersistentDataHandler implements PersistentDataInterface


what about InMemoryPersistentDataHandler here? I feel it's more common

yguedidi · 2017-12-03T19:47:47Z

tests/AppTest.php

 use Facebook\Authentication\AccessToken;
 use PHPUnit\Framework\TestCase;

-class FacebookAppTest extends TestCase
+class AppTest extends TestCase


then ApplicationTest there if you agree on the tested class name :)

yguedidi · 2017-12-03T19:48:44Z

tests/PersistentData/MemoryPersistentDataHandlerTest.php

 use PHPUnit\Framework\TestCase;

-class FacebookMemoryPersistentDataHandlerTest extends TestCase
+class MemoryPersistentDataHandlerTest extends TestCase


then InMemoryPersistentDataHandlerTest here

yguedidi

You should add a commit that update names by "Find in Path..." so PHPDocs and the doc get updated too ;)
So this PR will become bigger haha

Nyholm · 2017-12-03T22:02:41Z

Thank you for the review. I've fixed your comments and I've updated the docs

yguedidi

One last request, as you updated PHPDocs, can you fix tag, type, and description alignment please?

yguedidi · 2017-12-04T00:12:56Z

@SammyK can you have a look at this PR please? thanks

Nyholm · 2017-12-04T08:40:22Z

I've aligned the doc blocks and made sure they follow PSR5 with some help from PHPcs fixer.

yguedidi · 2017-12-04T11:41:55Z

Thanks a lot for PHPCSFixer!!!! Looks like I merged a PR that create lots of conflicts with yours... Can you please rebase on latest master?

Nyholm · 2017-12-04T21:42:59Z

The PR is rebased..

Nyholm · 2017-12-04T21:43:58Z

Oh... new conflicts.. Rebasing again =)

Nyholm · 2017-12-04T22:14:05Z

There, I've rebased again. Please have a look

yguedidi

should be Application then in PHPDoc, please check everywhere else and also InMemory.. Thanks a lot

yguedidi · 2017-12-04T23:24:39Z

src/Application.php

@@ -87,7 +86,7 @@ public function getAccessToken()
    }

    /**
-     * Serializes the FacebookApp entity as a string.
+     * Serializes the App entity as a string.


Application

yguedidi · 2017-12-04T23:24:56Z

src/Application.php

@@ -97,7 +96,7 @@ public function serialize()
    }

    /**
-     * Unserializes a string as a FacebookApp entity.
+     * Unserializes a string as a App entity.


Application

yguedidi · 2017-12-04T23:25:13Z

src/Authentication/OAuth2Client.php

@@ -44,16 +41,16 @@ class OAuth2Client
    const BASE_AUTHORIZATION_URL = 'https://www.facebook.com';

    /**
-     * The FacebookApp entity.
+     * The App entity.


Application

yguedidi · 2017-12-04T23:25:33Z

src/BatchRequest.php

@@ -114,26 +112,26 @@ public function add($request, $options = null)
    }

    /**
-     * Ensures that the FacebookApp and access token fall back when missing.
+     * Ensures that the App and access token fall back when missing.


Application

yguedidi · 2017-12-04T23:25:48Z

src/BatchRequest.php

    {
        if (!$request->getApp()) {
            $app = $this->getApp();
            if (!$app) {
-                throw new FacebookSDKException('Missing FacebookApp on FacebookRequest and no fallback detected on FacebookBatchRequest.');
+                throw new SDKException('Missing App on Request and no fallback detected on BatchRequest.');


Application

yguedidi · 2017-12-04T23:26:01Z

src/Helper/PageTabHelper.php

-     * @param FacebookApp    $app          The FacebookApp entity.
-     * @param FacebookClient $client       The client to make HTTP requests.
-     * @param string         $graphVersion The version of Graph to use.
+     * @param Application $app          the App entity


Application

yguedidi · 2017-12-04T23:26:16Z

src/Helper/SignedRequestFromInputHelper.php

     */
    protected $signedRequest;

    /**
-     * @var FacebookApp The FacebookApp entity.
+     * @var Application the App entity


Application

yguedidi · 2017-12-04T23:26:24Z

src/Helper/SignedRequestFromInputHelper.php

-     * @param FacebookApp    $app          The FacebookApp entity.
-     * @param FacebookClient $client       The client to make HTTP requests.
-     * @param string         $graphVersion The version of Graph to use.
+     * @param Application $app          the App entity


Application

yguedidi · 2017-12-04T23:48:50Z

I'm really sorry @Nyholm, I did a mistake when merging 5.x into master, so tests were broken (looks like I'm tired..), so you will need to rebase again, sorry

Nyholm · 2017-12-05T07:24:24Z

Updated!

Nyholm · 2017-12-05T07:26:33Z

Wohoo tests are green!

yguedidi · 2017-12-05T12:17:06Z

Thanks a lot @Nyholm!!

Nyholm · 2017-12-05T15:00:09Z

Awesome. Thanks!

Nyholm commented Dec 2, 2017

View reviewed changes

facebook-github-bot added the CLA Signed label Dec 2, 2017

yguedidi reviewed Dec 3, 2017

View reviewed changes

yguedidi suggested changes Dec 3, 2017

View reviewed changes

yguedidi suggested changes Dec 4, 2017

View reviewed changes

yguedidi requested a review from SammyK December 4, 2017 00:12

Nyholm force-pushed the prefix branch from ea47ffc to 0c9c3b0 Compare December 4, 2017 21:34

Nyholm force-pushed the prefix branch from f60a69b to fb1bee3 Compare December 4, 2017 21:55

yguedidi force-pushed the master branch from 4720993 to db9c1fe Compare December 4, 2017 22:04

Nyholm force-pushed the prefix branch from 68969e3 to a920fb1 Compare December 4, 2017 22:13

yguedidi suggested changes Dec 4, 2017

View reviewed changes

Nyholm added 6 commits December 5, 2017 08:15

Removed "Facebook" prefix form all classes

2d6f192

code style fixes

efd5963

merge fix

16747c9

Doc fixes

665a6f1

Fixed broken tests

2cb0494

Did some more renaming

dda97c2

Nyholm force-pushed the prefix branch from a920fb1 to dda97c2 Compare December 5, 2017 07:22

yguedidi approved these changes Dec 5, 2017

View reviewed changes

Merge branch 'master' into prefix

0c9d3f0

yguedidi merged commit 8875d3c into facebookarchive:master Dec 5, 2017

yguedidi mentioned this pull request Dec 5, 2017

Remove the Facebook prefix from nearly all classes #609

Closed

Nyholm deleted the prefix branch December 5, 2017 15:00


		class FacebookApp implements \Serializable
		class App implements \Serializable

Removed "Facebook" prefix form all classes #888

Removed "Facebook" prefix form all classes #888

Uh oh!

Conversation

Nyholm commented Dec 2, 2017

Uh oh!

Nyholm left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

yguedidi left a comment

Choose a reason for hiding this comment

Uh oh!

Nyholm commented Dec 3, 2017

Uh oh!

yguedidi left a comment

Choose a reason for hiding this comment

Uh oh!

yguedidi commented Dec 4, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Nyholm commented Dec 4, 2017

Uh oh!

yguedidi commented Dec 4, 2017

Uh oh!

Nyholm commented Dec 4, 2017

Uh oh!

Nyholm commented Dec 4, 2017

Uh oh!

Nyholm commented Dec 4, 2017

Uh oh!

yguedidi left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

yguedidi commented Dec 4, 2017

Uh oh!

Nyholm commented Dec 5, 2017

Uh oh!

Nyholm commented Dec 5, 2017

Uh oh!

yguedidi commented Dec 5, 2017

Uh oh!

Nyholm commented Dec 5, 2017

Uh oh!

Uh oh!

yguedidi commented Dec 4, 2017 •

edited

Loading