Skip to content
This repository has been archived by the owner on Dec 23, 2017. It is now read-only.

Moving to Flask #22

Merged
merged 66 commits into from
Feb 2, 2015
Merged

Moving to Flask #22

merged 66 commits into from
Feb 2, 2015

Conversation

theresaanna
Copy link
Contributor

Sorry, this is a giant one.

Removing almost all of the client side app. Leaving behind pieces that deal with filter formatting and activation, and Typeahead. (Which works, btw. The back end just times out sometimes, presumably because certain requests take too long. I imagine "Obama" and "Romney" like we're all trying have a bazillion committees to pull totals for. Try "pelosi" and choose Nancy Pelosi and her first committee.)

I've only successfully written tests for the openfecwebapp.data_mappings module. Maybe I'm just tired and have been working too long, but I can't quite figure out how to test the views and api_caller modules without headaches. I am happily taking suggestions.

I really want to remove the load_totals calls from openfecwebapp.views.render_page but am not sure what is best. Again, would like feedback.

Also includes some of @noahmanger's markup and css work.

The JS version is frozen in https://github.com/18F/openFEC-web-app/tree/js-version.

candidate['joint_committees'] = []

for i in range(len(candidate['affiliated_committees'])):
cmte = candidate['affiliated_committees'][i]
Copy link
Contributor

Choose a reason for hiding this comment

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

Might be easier to read if you iterate over the elements rather than the index:

for cmte in candidate['affiliated_committees']:

@cmc333333
Copy link
Contributor

It's amazing how easy you make porting this over seem, @theresaanna . If you all want to keep mustache/handlebar/etc. templates, you can; you'll just need to find the Python library that implements them instead of using jinja.

I made a bunch of style comments, but they are all about tightening things up. In general, try to add docstrings to your functions rather than # comments. Also, remember that you can iterate over the elements of a collection rather than the index. If you really need the index, you can

for idx, value in enumerate(your_list)

@theresaanna
Copy link
Contributor Author

@cmc333333 I looked at the available Flask + Handlebars libraries and they were sparse. I know I could use the regular Python Handlebars library, but I wanted to keep things very clean and Flask, and I didn't want to put in the effort to figure it out, tbh. I think Jinja is nice, and I'm glad to have the concept of template blocks now.

Thank you for your review. I suspected many things could be more pythonic, so this is great feedback.

cmc333333 added a commit that referenced this pull request Feb 2, 2015
@cmc333333 cmc333333 merged commit b184fc7 into master Feb 2, 2015
@theresaanna theresaanna deleted the flask branch February 9, 2015 19:11
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants