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

Addition of twitter feed #41

Merged
merged 1 commit into from
Dec 14, 2017
Merged

Addition of twitter feed #41

merged 1 commit into from
Dec 14, 2017

Conversation

dob9601
Copy link
Contributor

@dob9601 dob9601 commented Dec 12, 2017

Addition of twitter feed to main page along with patch to fix the favicon and org logo

Closes #40

api_data_dump = json.loads(
requests.get('https://gci-leaders.netlify.com/data.json').content)
for item in api_data_dump:
if item["name"] == org_name:
Copy link
Collaborator

Choose a reason for hiding this comment

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

You do not use the preferred quotation marks.

Origin: QuotesBear, Section: all.python.

The issue can be fixed by applying the following patch:

--- a/tmp/tmplwzmo6yi/gci/view_overview.py
+++ b/tmp/tmplwzmo6yi/gci/view_overview.py
@@ -42,7 +42,7 @@
         api_data_dump = json.loads(
             requests.get('https://gci-leaders.netlify.com/data.json').content)
         for item in api_data_dump:
-            if item["name"] == org_name:
+            if item['name'] == org_name:
                 org_twitter_handle = item["twitter_url"].split(
                     "twitter.com/")[-1]
         if org_twitter_handle is not None:

requests.get('https://gci-leaders.netlify.com/data.json').content)
for item in api_data_dump:
if item["name"] == org_name:
org_twitter_handle = item["twitter_url"].split(
Copy link
Collaborator

Choose a reason for hiding this comment

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

You do not use the preferred quotation marks.

Origin: QuotesBear, Section: all.python.

The issue can be fixed by applying the following patch:

--- a/tmp/tmplwzmo6yi/gci/view_overview.py
+++ b/tmp/tmplwzmo6yi/gci/view_overview.py
@@ -43,7 +43,7 @@
             requests.get('https://gci-leaders.netlify.com/data.json').content)
         for item in api_data_dump:
             if item["name"] == org_name:
-                org_twitter_handle = item["twitter_url"].split(
+                org_twitter_handle = item['twitter_url'].split(
                     "twitter.com/")[-1]
         if org_twitter_handle is not None:
             s.append('<a class="twitter-timeline" data-height="1000" '

for item in api_data_dump:
if item["name"] == org_name:
org_twitter_handle = item["twitter_url"].split(
"twitter.com/")[-1]
Copy link
Collaborator

Choose a reason for hiding this comment

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

You do not use the preferred quotation marks.

Origin: QuotesBear, Section: all.python.

The issue can be fixed by applying the following patch:

--- a/tmp/tmplwzmo6yi/gci/view_overview.py
+++ b/tmp/tmplwzmo6yi/gci/view_overview.py
@@ -44,7 +44,7 @@
         for item in api_data_dump:
             if item["name"] == org_name:
                 org_twitter_handle = item["twitter_url"].split(
-                    "twitter.com/")[-1]
+                    'twitter.com/')[-1]
         if org_twitter_handle is not None:
             s.append('<a class="twitter-timeline" data-height="1000" '
                      'data-link-color="#2B7BB9" '

@@ -8,7 +8,7 @@
from django.views.generic import TemplateView

from gci.views import index as gci_index

from gci.view_overview import index as overview_index
Copy link
Collaborator

Choose a reason for hiding this comment

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

The code does not comply to PEP8.

Origin: PEP8Bear, Section: all.autopep8.

The issue can be fixed by applying the following patch:

--- a/tmp/tmplwzmo6yi/community/urls.py
+++ b/tmp/tmplwzmo6yi/community/urls.py
@@ -9,6 +9,7 @@
 
 from gci.views import index as gci_index
 from gci.view_overview import index as overview_index
+
 
 def get_index():
     # The index URI regex, ^$, contains no parameters, named or otherwise.

@@ -8,7 +8,7 @@
from django.views.generic import TemplateView

from gci.views import index as gci_index

from gci.view_overview import index as overview_index

def get_index():
Copy link
Collaborator

Choose a reason for hiding this comment

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

E302 expected 2 blank lines, found 1

Origin: PycodestyleBear (E302), Section: all.autopep8.

@jayvdb
Copy link
Member

jayvdb commented Dec 13, 2017

ack 5e6defe

@jayvdb
Copy link
Member

jayvdb commented Dec 13, 2017

unack 60225f5

Copy link
Member

@jayvdb jayvdb left a comment

Choose a reason for hiding this comment

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

Restructure and simply.

gci/views.py Outdated
@@ -44,8 +44,8 @@ def gci_overview():
org_logo_file.write(org_logo)

s.append('<link rel="shortcut icon" type="image/png" '
'href="static/favicon.png"/>')
s.append('<img src="static/org_logo.png" alt="'+org_name+'">')
'href="../static/favicon.png"/>')
Copy link
Member

Choose a reason for hiding this comment

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

After looking at your patch, I think we need to do the logo as a separate PR, and it should wait until we've done some other improvements to the infrastructure.
I've created an issue for it at #42

@@ -18,7 +19,7 @@ def get_index():

urlpatterns = [
distill_url(
r'^$', TemplateView.as_view(template_name='index.html'),
r'^$', overview_index,
Copy link
Member

Choose a reason for hiding this comment

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

with this change, the file templates/index.html becomes unused. But we need to be using more raw html & templates, and less code generated HTML (ugly).

Instead of replacing index.html I think it would be better for the twitter feed to be on /twitter/ , in a new app in /twitter directory (like activity)

Then the /twitter link can be added to templates/index.html .

api_data_dump = json.loads(
requests.get('https://gci-leaders.netlify.com/data.json').content)
for item in api_data_dump:
if item['name'] == org_name:
Copy link
Member

Choose a reason for hiding this comment

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

The new twitter component can load org_name.txt which is created at https://github.com/coala/community/blob/master/.ci/build.sh#L5 and https://github.com/coala/community/blob/master/orgname.sh#L5

Then it does not need org_name = linked_students[0]['organization_name'] and all that mess I wrote last week (I am rewriting it!)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

that's useful, so the same can be done with the org logo at some stage so that it isn't reliable on api access

@jayvdb
Copy link
Member

jayvdb commented Dec 13, 2017

unack 4eaf47f

def index(request):
s = []

org_name = open("org_name.txt", "r").read()
Copy link
Collaborator

Choose a reason for hiding this comment

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

You do not use the preferred quotation marks.

Origin: QuotesBear, Section: all.python.

The issue can be fixed by applying the following patch:

--- a/tmp/tmpxvfdx0l0/gci/view_twitter.py
+++ b/tmp/tmpxvfdx0l0/gci/view_twitter.py
@@ -8,7 +8,7 @@
 def index(request):
     s = []
 
-    org_name = open("org_name.txt", "r").read()
+    org_name = open('org_name.txt', "r").read()
     favicon = get_logo(org_name, 16)
     with open('_site/favicon.png', 'wb') as favicon_file:
         favicon_file.write(favicon)

def index(request):
s = []

org_name = open("org_name.txt", "r").read()
Copy link
Collaborator

Choose a reason for hiding this comment

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

You do not use the preferred quotation marks.

Origin: QuotesBear, Section: all.python.

The issue can be fixed by applying the following patch:

--- a/tmp/tmpxvfdx0l0/gci/view_twitter.py
+++ b/tmp/tmpxvfdx0l0/gci/view_twitter.py
@@ -8,7 +8,7 @@
 def index(request):
     s = []
 
-    org_name = open("org_name.txt", "r").read()
+    org_name = open("org_name.txt", 'r').read()
     favicon = get_logo(org_name, 16)
     with open('_site/favicon.png', 'wb') as favicon_file:
         favicon_file.write(favicon)

'href="../static/favicon.png"/>')
s.append('<img src="../static/org_logo.png" alt="'+org_name+'">')


Copy link
Collaborator

Choose a reason for hiding this comment

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

The code does not comply to PEP8.

Origin: PEP8Bear, Section: all.autopep8.

The issue can be fixed by applying the following patch:

--- a/tmp/tmpxvfdx0l0/gci/view_twitter.py
+++ b/tmp/tmpxvfdx0l0/gci/view_twitter.py
@@ -20,8 +20,6 @@
              'href="../static/favicon.png"/>')
     s.append('<img src="../static/org_logo.png" alt="'+org_name+'">')
 
-
-
     api_data_dump = json.loads(
         requests.get('https://gci-leaders.netlify.com/data.json').content)
     for item in api_data_dump:




api_data_dump = json.loads(
Copy link
Collaborator

Choose a reason for hiding this comment

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

E303 too many blank lines (3)

Origin: PycodestyleBear (E303), Section: all.autopep8.

@dob9601 dob9601 force-pushed the twitter branch 2 times, most recently from 3686b5d to 9f27d98 Compare December 13, 2017 18:09
def index(request):
s = []

org_name = open("org_name.txt", "r").read()
Copy link
Collaborator

Choose a reason for hiding this comment

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

You do not use the preferred quotation marks.

Origin: QuotesBear, Section: all.python.

The issue can be fixed by applying the following patch:

--- a/tmp/tmp7kw0kfov/gci/view_twitter.py
+++ b/tmp/tmp7kw0kfov/gci/view_twitter.py
@@ -8,7 +8,7 @@
 def index(request):
     s = []
 
-    org_name = open("org_name.txt", "r").read()
+    org_name = open('org_name.txt', "r").read()
     favicon = get_logo(org_name, 16)
     with open('_site/favicon.png', 'wb') as favicon_file:
         favicon_file.write(favicon)

def index(request):
s = []

org_name = open("org_name.txt", "r").read()
Copy link
Collaborator

Choose a reason for hiding this comment

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

You do not use the preferred quotation marks.

Origin: QuotesBear, Section: all.python.

The issue can be fixed by applying the following patch:

--- a/tmp/tmp7kw0kfov/gci/view_twitter.py
+++ b/tmp/tmp7kw0kfov/gci/view_twitter.py
@@ -8,7 +8,7 @@
 def index(request):
     s = []
 
-    org_name = open("org_name.txt", "r").read()
+    org_name = open("org_name.txt", 'r').read()
     favicon = get_logo(org_name, 16)
     with open('_site/favicon.png', 'wb') as favicon_file:
         favicon_file.write(favicon)

'href="../static/favicon.png"/>')
s.append('<img src="../static/org_logo.png" alt="'+org_name+'">')


Copy link
Collaborator

Choose a reason for hiding this comment

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

The code does not comply to PEP8.

Origin: PEP8Bear, Section: all.autopep8.

The issue can be fixed by applying the following patch:

--- a/tmp/tmp7kw0kfov/gci/view_twitter.py
+++ b/tmp/tmp7kw0kfov/gci/view_twitter.py
@@ -20,8 +20,6 @@
              'href="../static/favicon.png"/>')
     s.append('<img src="../static/org_logo.png" alt="'+org_name+'">')
 
-
-
     api_data_dump = json.loads(
         requests.get('https://gci-leaders.netlify.com/data.json').content)
     for item in api_data_dump:




api_data_dump = json.loads(
Copy link
Collaborator

Choose a reason for hiding this comment

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

E303 too many blank lines (3)

Origin: PycodestyleBear (E303), Section: all.autopep8.

@dob9601 dob9601 force-pushed the twitter branch 3 times, most recently from 8b8f2a8 to e8f9e99 Compare December 13, 2017 18:17
Copy link
Member

@jayvdb jayvdb left a comment

Choose a reason for hiding this comment

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

this shouldnt be in /gci/ in the repo or in the website. It is a twitter feed of the entire org, not part of GCI. The code needs to be a separate app , in the repo at twitter/. And please refer to my previous review where I explain how to get the org_name .

@dob9601
Copy link
Contributor Author

dob9601 commented Dec 13, 2017

@jayvdb how would you want me to access the org_name differentl? Right now, in the view_twitter file i’m accessing it via the org_name.txt file.

gci/views.py Outdated
@@ -39,13 +39,8 @@ def gci_overview():
with open('_site/favicon.png', 'wb') as favicon_file:
favicon_file.write(favicon)

org_logo = get_logo(org_name)
Copy link
Member

Choose a reason for hiding this comment

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

please dont modify the logo stuff. That is a different task

def index(request):
s = []

org_name = open('org_name.txt', 'r').read()
Copy link
Member

Choose a reason for hiding this comment

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

ah, I see this now. that is ok.

Copy link
Member

Choose a reason for hiding this comment

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

but strip() the contents from the file to remove line feeds etc

import requests
import json

from .gitorg import get_logo
Copy link
Member

Choose a reason for hiding this comment

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

no logo. twitter only. dont depend on anything in gci/

@dob9601 dob9601 force-pushed the twitter branch 2 times, most recently from eac4d84 to 2ea90b0 Compare December 13, 2017 21:39
@@ -8,6 +8,7 @@
from django.views.generic import TemplateView

from gci.views import index as gci_index
from gci.view_twitter import index as twitter_index
Copy link
Member

Choose a reason for hiding this comment

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

this doesnt compile. gci.view_twitter doesnt exist

Copy link
Member

@jayvdb jayvdb Dec 14, 2017

Choose a reason for hiding this comment

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

you can see this on the travis build: https://travis-ci.org/coala/community/builds/316110663#L807

ModuleNotFoundError: No module named 'gci.view_twitter'

Copy link
Contributor Author

Choose a reason for hiding this comment

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

sorry, stupid mistake. Will fix it

@jayvdb jayvdb merged commit 3080184 into coala:master Dec 14, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

Organisation Twitter feed
3 participants