Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP

Loading…

Bug 663572 Bulk Date Upload #2

Merged
merged 4 commits into from

2 participants

@Osmose
Owner

Adds a bulk date upload action and view for changing the dates of multiple snippets at once.

@davedash

we usually do URL reversing for URLs - it ensures that every URL is an actual one.

Also, this URL seems weird, we usually do _ in urls, not camelcase.

@davedash

Better specifiy what things your importing if you can. If it's a lot of things, you can:

from django import forms

and prefix things with forms.

@davedash

Run all your code thorugh check.py before committing.

@davedash

<backspace><backspace>

@davedash

this doesn't fit in <80 chars on one line?

@davedash

This line is too long

Make use of python's awesomeness:

url(r'^(?P<startpage_version>[^/]+)/(?P<name>[^/]+)/(?P<version>[^/]+)/'
  '(?P<appbuildid>[^/]+)/(?P<build_target>[^/]+)/(?P<locale>[^/]+)/'
  '(?P<channel>[^/]+)/(?P<os_version>[^/]+)/(?P<distribution>[^/]+)/'
  '(?P<distribution_version>[^/]+)/$',

No concatenation symbols needed, just multiple strings next to each other are treated as one big string.

@davedash

you said "staff"

... no wait you said "member"

Owner

Would you suggest aliasing the import? I think it's clearer to not change the function name from what's in the django lib.

no I just thought it was funny.

Owner

I need to get less paranoid about code reviews. :P

@davedash

Full sentences require a . at the end.

@davedash

lots of style issues, go through this with check.py and I've highlighted a few styly things.

I assume the code works :)

@davedash

This seems redundant, I gathered that it's Form classes since this is Django and it's caleld forms.py.

@Osmose Osmose merged commit b75190d into mozilla:master
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Something went wrong with that request. Please try again.