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

Streaming Updates for v6 #631

Closed
wants to merge 122 commits into from
Closed

Streaming Updates for v6 #631

wants to merge 122 commits into from

Conversation

stve
Copy link
Collaborator

@stve stve commented Nov 26, 2014

Just getting started but wanted to enumerate the priorities. I'm sure I'm missing a few but here's some things that I think should be on the punchlist:

sferik and others added 26 commits November 8, 2014 12:29
caching is also possible, but if you do choose to add it, it is highly recommended to also add `bundle update` in a `before_script` section
use --retry and --jobs to bundle install faster
@sferik
Copy link
Owner

sferik commented Nov 26, 2014

@stve This looks like a good start and a solid checklist.

I would add one item:

  • 100% c0 code coverage for streaming

Currently, this library has 100% code coverage, except for one method: Twitter::Streaming::Connection#stream, which is faked in the Twitter::Streaming::Client specs. I was okay with less-than-complete code coverage when this was an experimental feature, but with the experimental warning label coming off in version 6, everything ought to be properly tested.

As always, let me know how I can be helpful. That said, I consider you to be the domain expert when it comes to Twitter Streaming, given your experience designing and maintaining em-twitter and TweetStream. If I had a clear idea of how to implement the items on the list above, I probably would have done it by now. I guess that’s another way of saying thanks so much for your help on this. I couldn’t do it without you. 😄

@stve stve self-assigned this Nov 26, 2014
@stve
Copy link
Collaborator Author

stve commented Nov 26, 2014

@sferik totally agree on test coverage. The changes I just committed brought coverage on Twitter::Streaming::Connection up to 87.88% (and overall coverage to 99.78%) so we are getting closer.

@sferik
Copy link
Owner

sferik commented Nov 26, 2014

@stve That’s great news!

@sferik
Copy link
Owner

sferik commented Sep 14, 2015

I rebased master (:scream:) leading up to the 5.15.0 release because I wanted to get some of the non-6.0.0-specific features and bug fixes out into the world (see the CHANGELOG for details), since it had been more than 6 months since the previous release. I was hoping 6.0.0 would be released by now but it’s been slower going than expected for a variety of reasons, many of which are out of our control (e.g. blockers in the http gem and extremely high churn in the Twitter Ads API). That means that 6.0.0 is probably not coming any time soon, unless we dramatically start cutting scope.

In the best-case scenario, I’ll get a large chunk of dedicated time to work on this library in December and we can ship around the New Year. Worst-case is probably mid-2016, at which point, I’ll agressively start cutting scope just to ship whatever’s ready and start working on adding non-breaking changes in 6.1.0 and breaking changes in 7.0.0.

The good news is there is a clean diff between the 5-stable and master branches: master is based on 5-stable and includes only the changes that are specific to version 6. The bad new is all other branches based on master need to be rebased. This will be a relatively painful rebase (~50 commits). I apologize in advance for making you go through that, but I think it will be worth having a clean separation between the branches as we push toward a 6.0.0 release.

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.

None yet