Skip to content
This repository has been archived by the owner on Nov 9, 2017. It is now read-only.

small changes #45

Open
wants to merge 6 commits into
base: master
Choose a base branch
from
Open

small changes #45

wants to merge 6 commits into from

Conversation

bamarni
Copy link

@bamarni bamarni commented Feb 5, 2013

No description provided.

"target-dir": "FOS/TwitterBundle",
"extra": {
"branch-alias": {
"dev-master": "1.1.x-dev"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

wrong. Should be 1.2.x-dev as latest release is alreayd 1.2.2

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@stof : Are you sure? I can only see a 1.0 branch with one release on it, so I assumed the current master is 1.1.x-dev.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sorry, I got confused by looking at too many PRs in different bundles in a short time and looked at the wrong package on Packagist

@@ -9,7 +9,6 @@

<xsd:complexType name="config">
<xsd:attribute name="alias" type="xsd:string" />
<xsd:attribute name="file" type="xsd:string" />
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what is the point of keeping it in the Configuration class to avoid breaking things for old configs if you break them before that when validating the schema for people using XML ?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you're right, I've added a note instead.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

but it could also have been a way to make people see the deprecation... usually IDEs show some warning on that situation.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the goal of deprecating instead of removing is to provide BC. An XML validation error is not BC (and hard to understand for many people, especially when the same stuff was workign previously)

@@ -40,7 +40,7 @@ public function getConfigTreeBuilder()
})
->end()
->children()
->scalarNode('file')->defaultValue('%kernel.root_dir%/../vendor/twitteroauth/twitteroauth/twitteroauth.php')->end()
->scalarNode('file')->info('DEPRECATED. To be removed.')->end()
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is bumping the requirement to Symfony 2.1 so please update the composer.json file

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what a catch! fixed

@stof
Copy link
Member

stof commented Apr 27, 2013

Can you also modify the .travis.yml file to test with current Symfony instead of 2.0 ?

"kertz/twitteroauth": "*"
},
"minimum-stability": "dev",
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

because "kertz/twitteroauth" has no stable version cc @Kertz

@lemoinem
Copy link

lemoinem commented Sep 3, 2013

Any news regarding this? Would be nice to have the package work out of the box...

@stof
Copy link
Member

stof commented Sep 3, 2013

my opinion is to officially deprecate this bundle as most of its code is about features dropped by Twitter anyway. The only remaining feature in the bundle is the OAuth 1 integration, and HWIOAuthBundle is a better choice for it IMO.

@arghav
Copy link
Member

arghav commented Sep 3, 2013

I agree with @stof, it doesn't make much sense to maintain this bundle anymore when HWIOAuthBundle pretty much does the same thing.

That said, right now I do not have the time to maintain the repo kertz/twitteroauth. If anyone is ready to maintain it I can transfer the ownership. Possibly to FOS itself if it makes sense.

@lemoinem
Copy link

lemoinem commented Sep 3, 2013

Well, In my case, I just need access to the Twitter API (Not really the sign-in with twitter, yet)...
In respect to this, themattharris/tmhOAuth seems to be a bit more up-to-date (e.g. at least support streaming API which seems fairly recent). And seems easy enough to use.

As far as I can see, HWIOAuthBundle doesn't provide access to the Twitter API (nor is it even remotely close to its goal).

Do you think it would be a good idea to simply provide a new version of this Bundle which would be a wrapper around tmhOAuth so we can have an easy to use bundle providing access to the Twitter API (Just like FOSFacebook does with the Facebook PHP SDK ?)

@stof
Copy link
Member

stof commented Sep 3, 2013

@Kertz I don't think it makes sense to transfer it to FOS. We already don't have enough time to maintain the current set of repos. and once this bundle get deprecated officially in favor of HWIOAuthBundle, the FOS bundles will not depend on the library anymore. Thus, AFAIK, none of the FOS devs is using the FOSTwitterBundle currently (which is also one of the reason why it looks abandoned already)

@lemoinem If you want to access the API on your own behalf, FOSTwitterBundle is not close either. If you want to access the API on behalf of your user, you need 2 different things: first authenticating through OAuth (which is what HWIOAuthBundle and FOSTwitterBundle are doing) and then using the retrieved access token in an API client. I don't think it makes sense to create a bundle doing only $apiClient->setToken($hwiToken->getAccessToken()).
And regarding the comparison with FOSFacebookBundle, my own preference would be to deprecate FOSFacebookBundle as well. Currently, it implements the OAuth2 auth through the Facebook SDK, which is a huge piece of crap (actually, the bundle had to rewrote 2/3 of the code to be able to use the Symfony Session system) and is the reason of most bugs reported on it. Thus, the implementation of HWIOauthBundle is more efficient, not only cleaner. So if I had to do some Facebook API calls on behalf of a user (I really hope I won't), I would still do it by using HWIOAuthBundle for the auth part, as shown above.

@lemoinem
Copy link

lemoinem commented Sep 4, 2013

Thanks @stof for the precision. After a couple of tests, FOSTwitter is most definitely not what I need...
👍 for deprecation then...

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants