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

Update first_timers.py #118

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
76 changes: 13 additions & 63 deletions first_timers/first_timers.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,15 +9,14 @@
# Set up logging
logging.basicConfig(filename='app.log', level=logging.INFO)


DAYS_OLD = 15
MAX_TWEETS_LEN = 280
ELLIPSE = u'…'

ellipse = u'…'
api = 'https://api.github.com/search/issues'
FIRST_ISSUE_QUERY_URL = api + '?q=label:"{}"+is:issue+is:open&sort=updated&order=desc'
API = 'https://api.github.com/search/issues'
FIRST_ISSUE_QUERY_URL = API + '?q=label:"{}"+is:issue+is:open&sort=updated&order=desc'

# Logging helper function
# Logging helper functions
def log_info(message):
Comment on lines +19 to 20
Copy link
Owner

Choose a reason for hiding this comment

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

I think we should get rid of the log comment. I do not see any value from the comment.

Copy link
Author

Choose a reason for hiding this comment

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

ohh okay

Copy link
Owner

Choose a reason for hiding this comment

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

I don't think this is resolved.

logging.info(message)

Expand All @@ -27,18 +26,14 @@ def log_warning(message):
def log_error(message):
logging.error(message)


def humanize_url(api_url: str) -> str:
"""Make an API endpoint to a Human endpoint."""
match = re.match(
'https://api.github.com/repos/(.*)/(.*)/issues/([0-9]*)', api_url)
"""Make an API endpoint a Human endpoint."""
match = re.match('https://api.github.com/repos/(.*)/(.*)/issues/([0-9]*)', api_url)
if not match:
raise ValueError(f'Format of API URLs has changed: {api_url}')
user, repo, issue_num = match.group(1, 2, 3)

return f'https://github.com/{user}/{repo}/issues/{issue_num}'


def get_first_timer_issues(issue_label: str) -> list:
"""Fetches the first page of issues with the label first-timers-label
which are still open.
Expand All @@ -53,12 +48,10 @@ def get_first_timer_issues(issue_label: str) -> list:

return items


def check_days_passed(date_created: str, days: int) -> bool:
created_at = datetime.strptime(date_created, "%Y-%m-%dT%H:%M:%SZ")
return (datetime.now() - created_at).days < days


def add_repo_languages(issues):
"""Adds the repo languages to the issues list."""
for issue in issues:
Expand All @@ -70,22 +63,18 @@ def add_repo_languages(issues):
if res.ok:
issue['languages'] = res.json()
else:
log_warning('Could not handle response: ' +
str(res) + ' from the API.')
log_warning('Could not handle response: ' + str(res) + ' from the API.')
return issues


def get_fresh(old_issue_list, new_issue_list):
"""Returns which issues are not present in the old list of issues."""
old_urls = {x['url'] for x in old_issue_list}
return [x for x in new_issue_list if x['url'] not in old_urls]


def tweet_issues(issues, creds, debug=False):
"""Takes a list of issues and credentials and tweets through the account
associated with the credentials.
Also takes a parameter 'debug', which can prevent actual tweeting.
Returns a list of tweets.
associated with the credentials. Also takes a parameter 'debug', which can
Copy link
Owner

Choose a reason for hiding this comment

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

We do not need this change. The comment from Also takes a parameter .... is expected to be on the second line. Can you revert this back

Copy link
Author

Choose a reason for hiding this comment

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

yup fs

prevent actual tweeting. Returns a list of tweets.
"""
if len(issues) == 0:
return []
Expand All @@ -112,52 +101,13 @@ def tweet_issues(issues, creds, debug=False):
title = issue['title']

if len(title) > allowed_title_len:
title = title[: allowed_title_len - 1] + ellipse
title = title[:allowed_title_len - 1] + ELLIPSE
else:
if 'languages' in issue:
language_hashTags = ''.join(
' #{}'.format(lang) for lang in issue['languages']
)
language_hashTags = ''.join(' #{}'.format(lang) for lang in issue['languages'])
Copy link
Owner

Choose a reason for hiding this comment

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

Same as above, revert this back. It is easier to read it when it is multi line

Copy link
Author

Choose a reason for hiding this comment

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

will do it then

hashTags = hashTags + language_hashTags

max_hashtags_len = MAX_TWEETS_LEN - \
(url_len + 1) - (len(title) + 1)
max_hashtags_len = MAX_TWEETS_LEN - (url_len + 1) - (len(title) + 1)

if len(hashTags) > max_hashtags_len:
hashTags = hashTags[:max_hashtags_len - 1] + ellipse

url = humanize_url(issue['url'])

try:
# Encoding here because .format will fail with Unicode characters.
tweet = '{title} {url} {tags}'.format(
title=title,
url=url,
tags=hashTags
)

if not debug:
api.update_status(tweet)

tweets.append({
'error': None,
'tweet': tweet
})

log_info('Tweeted issue: {}'.format(issue['title']))
except Exception as e:
tweets.append({
'error': e,
'tweet': tweet
})

log_error('Error tweeting issue: {}'.format(issue['title']))
log_error('Error message: {}'.format(str(e)))

return tweets


def limit_issues(issues, limit_len=100000):
"""Limit the number of issues saved in our DB."""
sorted_issues = sorted(issues, key=lambda x: x['updated_at'], reverse=True)
return sorted_issues[:limit_len]
hashTags = hashTags[:max_hashtags_len - 1] + E
Copy link
Owner

Choose a reason for hiding this comment

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

Seems like you have deleted all the code after this line. Can you revert this please.

Copy link
Author

Choose a reason for hiding this comment

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

okayy