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

remove separate twitter bundle; use exception trapping to only regist… #1571

Conversation

justinedelson
Copy link
Contributor

…er AdapterFactory when Twitter4J is available

…er AdapterFactory when Twitter4J is available
@justinedelson
Copy link
Contributor Author

Upon installation, if Twitter4J isn't available, you get a single log message:

com.adobe.acs.commons.twitter.impl.TwitterAdapterFactory Twitter4J Library not found. Not registering TwitterAdapterFactory.

Note that due to #1570, this isn't actually testable as-is.

Also, this pull request doesn't remove the min packaging. I think that warrants keeping around for legacy reasons.

@coveralls
Copy link

coveralls commented Nov 19, 2018

Coverage Status

Coverage increased (+0.03%) to 41.175% when pulling f3a1fdd on justinedelson:feature/better-twitter-loading into 700ac67 on Adobe-Consulting-Services:master.

@badvision
Copy link
Contributor

Code climate strikes again... 🤕

@badvision
Copy link
Contributor

badvision commented Nov 19, 2018

We're doing a major version bump so we could remove the min packaging now, but after 4.0.0 it will be a while before we get another option to consider it.

@joerghoh
Copy link
Contributor

+1 for removal of the twitter4j bundle, thus eliminating the need for the min bundle

@badvision
Copy link
Contributor

I'll merge this in once the confirmation build passes (which I expect it to)

@badvision badvision added this to the 4.0.0 milestone Nov 19, 2018
@badvision badvision merged commit 979121c into Adobe-Consulting-Services:master Nov 19, 2018
@kwin kwin mentioned this pull request Mar 26, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants