Merge lp:~leonardo-lsrocha/polly/trunk into lp:polly

Proposed by Leonardo Rocha
Status: Merged
Approved by: Conscious User
Approved revision: 398
Merge reported by: Conscious User
Merged at revision: not available
Proposed branch: lp:~leonardo-lsrocha/polly/trunk
Merge into: lp:polly
Diff against target: 37 lines (+19/-1)
1 file modified
src/polly/shortener.py (+19/-1)
To merge this branch: bzr merge lp:~leonardo-lsrocha/polly/trunk
Reviewer Review Type Date Requested Status
Conscious User Approve
Review via email: mp+101943@code.launchpad.net
To post a comment you must log in.
Revision history for this message
Conscious User (conscioususer) wrote :

Thanks for the contribution. Some things need to be redone before merging.

1) Remove the changes to strings and constants, they are not needed.

2) Follow the naming convention and change the class name to MIGREMEShortener.

review: Needs Resubmitting
Revision history for this message
Conscious User (conscioususer) wrote :

You forgot to change the class name in the super call. Also, after I tried to merge your branch locally, I noticed several formatting issues.

1) Please respect the spacing convention: three blank lines between classes and two blank lines between methods.

2) Never use tabs. One identation level is equivalent to four spaces.

3) Make sure all your blank lines are really blank, and not with spaces or tabs.

review: Needs Fixing
Revision history for this message
Conscious User (conscioususer) wrote :

You are still not following the spacing convention between classes, the new class is too close to the last method of the previous class. Please use three blank lines between them.

Also, I didn't notice this before, but please move the new class to below goo.gl. I think it's better to respect the previous ordering.

Finally, do not make new commits just to fix the errors. Make sure the corrected submission is like the original one.

review: Needs Fixing
Revision history for this message
Conscious User (conscioususer) wrote :

Again: do not make new commits just to fix the errors. Submit the changes in a single commit.

review: Needs Fixing
Revision history for this message
Leonardo Rocha (leonardo-lsrocha) wrote :

> Again: do not make new commits just to fix the errors. Submit the changes in a
> single commit.

Sorry, but I haven't understood how can I submit without commit to fix errors. Do I need to create a new branch?

Revision history for this message
Conscious User (conscioususer) wrote :

Use "bzr uncommit" to revert the last commit (in your case, you need to do this four times). Then make the new, superseding commit and push your branch with the --overwrite parameter.

lp:~leonardo-lsrocha/polly/trunk updated
398. By Leonardo Rocha

Implemented MigreMe shortener

Revision history for this message
Conscious User (conscioususer) wrote :

I'm really sorry for the delay, this request got lost in my mailbox.

The patch seems fine now.

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'src/polly/shortener.py'
2--- src/polly/shortener.py 2011-10-24 20:23:33 +0000
3+++ src/polly/shortener.py 2012-05-09 13:38:27 +0000
4@@ -182,6 +182,24 @@
5
6
7
8+class MIGREMEShortener(Shortener):
9+ def _process(self, data):
10+ return data[u'migre']
11+
12+
13+ def __init__(self):
14+ super(MIGREMEShortener, self).__init__(u'migre.me')
15+
16+ self.messages = {}
17+
18+
19+ def shorten(self, url):
20+ parameters = {b'url': url.encode(ENCODING)}
21+
22+ return self._request_get(b'http://migre.me/api.json', parameters)
23+
24+
25+
26 class ControllerBackend(object):
27 def _update_enabled(self):
28 enabled = self.enabled_setting.get_value()
29@@ -272,7 +290,7 @@
30
31 self.shorteners = OrderedDict()
32
33- for Class in [ISGDShortener, BITLYShortener, GOOGLShortener]:
34+ for Class in [ISGDShortener, BITLYShortener, GOOGLShortener, MIGREMEShortener]:
35 shortener = Class()
36
37 self.shorteners[shortener.name] = shortener

Subscribers

People subscribed via source and target branches