djangopackages-zz / djangopackages

Django Packages - a place to review Django apps, frameworks, and projects.
http://djangopackages.com
MIT License
53 stars 40 forks source link

Improved query performance, code readability, and site usability. #4

Closed taavi223 closed 14 years ago

taavi223 commented 14 years ago

Removed some old code that was no longer being used. Cleaned up some small typos. Improved code readability. Drastically reduced queries on grid pages. Added commit histories to grid pages. Made it so that logging in from anywhere on the site will now redirect you back to the page you were previously on (with the exception of the logout page). Fixed issue 27 on scaredofrabbits: grids are now sorted by usage. Also added "I use this buttons" to all grid detail pages.

pydanny commented 14 years ago

Mostly is quite awesome. However, there are several problems and issues:

  1. Don't use 'e' for element. Write it out please.
  2. Don't combine actions on a single line. It makes debugging more challenging for other people visiting your code.
  3. Issue #7 details a bug. We discovered it late and were too tired to fix. If you grep on 'taavi' or 'bug' you should find it. Let me know how it goes.

Cheers!

pydanny commented 14 years ago

I added a _buildgrids utility method to the grid.views module. It is called whenever a new grid_package or feature is added and it adds in all the necessary elements. This closes ticket #7.

taavi223 commented 14 years ago

Sorry about the bug. I tested that viewing and editing existing elements still worked, but I didn't think to try adding a new feature or package and then trying to edit one of those new elements. I think this shows that we need tests. :-)

As for the other issues, I've added a commit which should take care of the readability/debugging concerns. If I missed the particular lines you were talking about, let me know and I'll take care of them. (And those are definitely very good suggestions, by the way. I have a tendency to try to make things more concise than they need to be.)