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

Method for favoriting a tweet and then following user #64

Merged
merged 7 commits into from
Aug 26, 2015

Conversation

kennethjmyers
Copy link
Contributor

Created a new method, auto_fav_then_follow, which combines auto_fav and auto_follow into one request.

Also updated the readme

@rhiever
Copy link
Owner

rhiever commented Jul 30, 2015

Thank you @kenncann! I'm going to have to give this merge some thought. I'm a bit nervous that merging code like this will lead to code bloat, e.g., next we'll have a "auto reply then follow" method and so on. At the moment I think I'd prefer to keep these methods separate, although I'm open to being convinced otherwise.

@kennethjmyers
Copy link
Contributor Author

That makes sense @rhiever, I had thought about reorganizing some of the code to put most of auto_follow in a separate method so that the parts that I duplicated weren't repeated but I can see that adding up fast. I'll probably continue to make edits that suit my needs and make pull requests every now and then to see if you like it

@rhiever
Copy link
Owner

rhiever commented Jul 30, 2015

I wonder if we can abstract the code base such that the auto_follow, auto_fav, etc. methods act on the direct result of the search tweets function rather than calling the search tweets function themselves. That way, a auto_fav_then_follow function would look like (pseudocode):

searched_tweets = search_tweets('phrase')
auto_fav(searched_tweets)
auto_follow(searched tweets)

Similarly, the auto_fav function that the user calls could be a wrapper for the underlying functions:

def auto_fav_phrase(phrase, count):
    searched_tweets = search_tweets(phrase)
    auto_fav(searched_tweets)

That would prevent the code duplication that we see in this PR.

Is that a rework you'd be willing to hack at? Otherwise, I'll add it as an enhancement issue for future work.

@kennethjmyers
Copy link
Contributor Author

Yeah that makes a lot of sense I'll work on that

@rhiever
Copy link
Owner

rhiever commented Jul 30, 2015

👍

kennethjmyers and others added 5 commits July 31, 2015 09:06
Updated the methods based on their new names:
auto_fav ---> auto_fav_phrase
auto_rt ---> auto_rt_phrase
auto_follow ---> auto_follow_from_phrase

New methods:
auto_fav_phrase_then_follow
auto_rt_phrase_then_follow
@kennethjmyers
Copy link
Contributor Author

Ok @rhiever, I've made the updates you suggested, let me know what you think!

rhiever pushed a commit that referenced this pull request Aug 26, 2015
Method for favoriting a tweet and then following user
@rhiever rhiever merged commit 3e0dbe9 into rhiever:master Aug 26, 2015
@rhiever
Copy link
Owner

rhiever commented Aug 26, 2015

Great work! Thank you. 👍

@rhiever
Copy link
Owner

rhiever commented Aug 26, 2015

Gah, sorry about this @kenncann. When I was reviewing this code to send out an update, I realized that the API completely changed with this code change. All the current users would have to change their existing code to accommodate the new API if they upgraded to the latest version, and I'd imagine that would cause quite a few problems.

I think the solution to this would be to switch the functions around: auto_fav, auto_follow, etc. still accept the phrase, count, result_type, etc. as inputs. Those functions would execute the search_tweets then pass the searched_tweets list to underlying auto_fav_tweet_list, auto_follow_tweet_list, etc. functions.

That would then easily allow the addition of auto_fav_then_follow functions because they would be calling the underlying auto*_list functions.

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.

2 participants