impala-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Jim Apple (Code Review)" <ger...@cloudera.org>
Subject [Impala-ASF-CR](asf-site) Initial commit of the blog section of the Impala ASF website.
Date Fri, 04 Nov 2016 15:24:32 GMT
Jim Apple has posted comments on this change.

Change subject: Initial commit of the blog section of the Impala ASF website.
......................................................................


Patch Set 1:

(19 comments)

http://gerrit.cloudera.org:8080/#/c/4944/1//COMMIT_MSG
Commit Message:

This is a large commit, so I may take a few rounds to review all of it.


PS1, Line 16: work properly
"work properly if Pelican is to be used for non-blog pages -- a job"


PS1, Line 25: comments section
This is presumably not possible with a static site generator without disqus or embedding Facebook
comments or something with JS?


http://gerrit.cloudera.org:8080/#/c/4944/1/Makefile
File Makefile:

Line 1: PY?=python
This deserves comments.

Also, I assume it was not written by you (given the many make options we don't use). If so,
it needs a license header.


http://gerrit.cloudera.org:8080/#/c/4944/1/blog/README.md
File blog/README.md:

Line 5: 
extra blank line?


PS1, Line 20:  path/to/venv
What is this path? How do I find it?


PS1, Line 24: (impala_blog)
What is this?


Line 32: Articles are stored as markdown files in the ```blog_content/``` directory, and follow
a specific template. The name of the file is not important, as long as it's unique. You sould
probably create this file on a new working branch in the repo.
Long line, here and below


Line 108: Don't forget to ```deactivate``` your virtualenv when you're done working on the
blog.
How?


http://gerrit.cloudera.org:8080/#/c/4944/1/pelicanconf.py
File pelicanconf.py:

Line 1: #!/usr/bin/env python
licence header, please


Line 3: from __future__ import unicode_literals
Why?


Line 5: AUTHOR = u'Impala Dev'
Does this need to be customized?


Line 22: # Blogroll
These could be removed or more clearly explained


Line 32: DEFAULT_PAGINATION = 100
100 posts per page?


Line 38: # Uncomment following line if you want document-relative URLs when developing
Why not?


http://gerrit.cloudera.org:8080/#/c/4944/1/themes/impala_asf_site_theme/templates/base.html
File themes/impala_asf_site_theme/templates/base.html:

Line 2: <html lang="{{ DEFAULT_LANG }}">
It would be good if this file mentioned in an HTML comment at the top that the HTML is automatically
generated and should not be hand-edited.


http://gerrit.cloudera.org:8080/#/c/4944/1/themes/impala_asf_site_theme/templates/gosquared.html
File themes/impala_asf_site_theme/templates/gosquared.html:

Line 1: {% if GOSQUARED_SITENAME %}
What is this all about?


http://gerrit.cloudera.org:8080/#/c/4944/1/themes/impala_asf_site_theme/templates/page.html
File themes/impala_asf_site_theme/templates/page.html:

Line 12: 		Last updated: {{ page.locale_modified }}
We generally avoid tabs


http://gerrit.cloudera.org:8080/#/c/4944/1/themes/impala_asf_site_theme/templates/tag.html
File themes/impala_asf_site_theme/templates/tag.html:

What is this for?


-- 
To view, visit http://gerrit.cloudera.org:8080/4944
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Iaa578a70237fcc97589c667c17a70d3d6dad5ae1
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: asf-site
Gerrit-Owner: David Knupp <dknupp@cloudera.com>
Gerrit-Reviewer: Henry Robinson <henry@cloudera.com>
Gerrit-Reviewer: Jim Apple <jbapple@cloudera.com>
Gerrit-HasComments: Yes

Mime
View raw message