-
-
Notifications
You must be signed in to change notification settings - Fork 62
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
base: master
Are you sure you want to change the base?
Conversation
i have actually formatted the previous code
i have formatted the code |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for working on this.
I have left a few comments. Lets get these comments fixed before we can get this merged.
# Logging helper functions | ||
def log_info(message): |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ohh okay
There was a problem hiding this comment.
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.
first_timers/first_timers.py
Outdated
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 |
There was a problem hiding this comment.
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
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yup fs
first_timers/first_timers.py
Outdated
language_hashTags = ''.join( | ||
' #{}'.format(lang) for lang in issue['languages'] | ||
) | ||
language_hashTags = ''.join(' #{}'.format(lang) for lang in issue['languages']) |
There was a problem hiding this comment.
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
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
will do it then
first_timers/first_timers.py
Outdated
"""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 |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
okayy
pls review it and tell if anything is wrong
@AniketTripathi05 Let me know once you are done with the fixes. I will review again. |
i have actually formatted the previous code
Fixes #issuenumber
Check everything which applies
#h1