Thanks for great comments, Euan. See a few notes below..
On Jul 7, 6:20 am, "euan.godd...@googlemail.com"
<euan.godd...@gmail.com> wrote:
> Hi again,
>
> I've had a read over your blog tutorial and have the following
> suggestions:
>
> 1) Make it PEP-8 (http://www.python.org/dev/peps/pep-0008/) compliant
> - it's a lot easier to read.
I fixed some things, I'm a bit ambivalent about 79 char limit. I feel
that with monitors getting larger, I think a lot of people use wider
windows, even if you have two 100-char windows side by side, you'll
still have some space left on many monitors. And there's a lot less
wrapping going from 79 to 99.
I also think code like:
if: ..
else: ..
..
looks more readable in most cases (depending on how long the clauses
are), than:
if:
..
else:
..
..
In other words, I always try to add a line after this type of
construct. (I know I missed it in a few places in current tutorial.)
> 2) Except only the error that might occur in the paginatior example.
> Bare excepts are BAD.
True, will fix.
> 3) Maybe consider enabling the context processor that adds the current
> user to the context
That's something I haven't done before so I'm not sure how to do it.
> 4) Consider using @login_required since all your views will break for
> logged out users
Hmm, all views work fine for me when logged out..
> 5) Consider using {% url %} instead of hard-coding your URLs
I'm running into problems with this tag in some cases. For example, in
admin's template, if I say {% url dbe.blog %} or just {% blog %}, or
add '.urls' to either, it's unable to reverse and throws an error. The
same thing happens when I try to do {% blog.urls post.id %} for the
"comments" link that goes on front page. I haven't used reverse before
so I might be missing something here.. In all other links in this
tutorial, reverse works fine.
> 6) It isn't necessary to coerce the pk to an int as Django takes care
> of it in: Post.objects.get(pk=int(pk))
Thanks! will fix..
> 7) I think the naming of variables in the add_comment view are
> confusing. The comment object is given a single letter name "c", but
> the comment form is confusingly given the name "comment". I'd suggest
> using more descriptive names,
Actually comment form is called cf, and on save it returns comment
object. But you're right, it shouldn't be called 'c' at first, will
fix..
> 8) In the same section you assume that the POST data will always have
> the key. Either use form validation here or allow for the POST to have
> missing data.
True, will fix.
> 9) The mkmonth_list could largely be taken care of by the builtin
> calendar module
I don't see how this function can be simplified with calendar. I'm
pretty familiar with it, I used it in a few projects recently.
Calendar is mostly helpful for days / weeks.
>
> Hope you find this helpful.
Very much so! Keep 'em coming!
>
> Euan
--
You received this message because you are subscribed to the Google Groups
"Django users" group.
To post to this group, send email to django-us...@googlegroups.com.
To unsubscribe from this group, send email to
django-users+unsubscr...@googlegroups.com.
For more options, visit this group at
http://groups.google.com/group/django-users?hl=en.
