Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code | Sign in
(386)

Issue 29556601: Issue 5777 - Update crowdin interface (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
2 years, 5 months ago by tlucas
Modified:
2 years, 4 months ago
Visibility:
Public.

Description

Issue 5777 - Update crowdin interface

Patch Set 1 #

Patch Set 2 : Added comment #

Total comments: 11

Patch Set 3 : Outsource post / get #

Total comments: 8

Patch Set 4 : Refactoring postFiles, addressing comments #

Patch Set 5 : Purged obsolete distinguishing between crowdin_{get|post} #

Total comments: 15

Patch Set 6 : Always require a json-response #

Total comments: 9

Patch Set 7 : Addressed Dave's comments #

Patch Set 8 : Print actual HTTP Error Code on Exception #

Total comments: 6

Patch Set 9 : Addressed Vasily's comments #

Total comments: 1

Patch Set 10 : Belatedly addressed Sebastian's comments #

Total comments: 11

Patch Set 11 : Mimetype, PEP-8 #

Unified diffs Side-by-side diffs Delta from patch set Stats (+23 lines, -32 lines) Patch
M localeTools.py View 1 2 3 4 5 6 7 8 9 10 4 chunks +23 lines, -32 lines 0 comments Download

Messages

Total messages: 23
tlucas
Patch Set 1 Fixing Issue 5777 - Update crowdin interface * Let all Crowdin interfaces ...
2 years, 5 months ago (2017-09-26 09:47:04 UTC) #1
Vasily Kuznetsov
Hi Tristan, I like the your refactoring. The repetition of Crowdin URLs all over the ...
2 years, 5 months ago (2017-09-26 11:05:53 UTC) #2
tlucas
Patch Set 3 Addressed or commented Vasily's comments. https://codereview.adblockplus.org/29556601/diff/29556604/localeTools.py File localeTools.py (right): https://codereview.adblockplus.org/29556601/diff/29556604/localeTools.py#newcode103 localeTools.py:103: get.update({'key': ...
2 years, 5 months ago (2017-09-26 12:13:13 UTC) #3
Vasily Kuznetsov
Hi Tristan, I like this version even better. A couple more comments. Cheers, Vasily https://codereview.adblockplus.org/29556601/diff/29556604/localeTools.py ...
2 years, 5 months ago (2017-09-26 13:11:37 UTC) #4
tlucas
Patch Set 4 * renaming postFiles / letting it only prepare a body / headers ...
2 years, 5 months ago (2017-09-26 14:40:30 UTC) #5
tlucas
Patch Set 5 Adjuste crowdin_{get|post}, please see in comments. https://codereview.adblockplus.org/29556601/diff/29556656/localeTools.py File localeTools.py (right): https://codereview.adblockplus.org/29556601/diff/29556656/localeTools.py#newcode114 localeTools.py:114: ...
2 years, 5 months ago (2017-09-26 14:49:06 UTC) #6
Sebastian Noack
https://codereview.adblockplus.org/29556601/diff/29556656/localeTools.py File localeTools.py (left): https://codereview.adblockplus.org/29556601/diff/29556656/localeTools.py#oldcode327 localeTools.py:327: body = body.encode('utf-8') This seems unrelated, but I noticed ...
2 years, 5 months ago (2017-09-27 04:04:53 UTC) #7
tlucas
Patch Set 1 * Always require a json response (load json only if not otherwise ...
2 years, 5 months ago (2017-09-27 10:51:31 UTC) #8
tlucas
Hey guys, please note the typo in the last mail: it's supposed to be "Patch ...
2 years, 5 months ago (2017-09-27 10:52:31 UTC) #9
kzar
Some of these changes look kind of unrelated, but I guess there's no harm improving ...
2 years, 4 months ago (2017-09-28 09:48:30 UTC) #10
tlucas
Patch Set 7: * Added a docstring to getTranslations * addressed PEP-8 error Thanks for ...
2 years, 4 months ago (2017-09-28 10:48:40 UTC) #11
kzar
https://codereview.adblockplus.org/29556601/diff/29557586/localeTools.py File localeTools.py (right): https://codereview.adblockplus.org/29556601/diff/29557586/localeTools.py#newcode128 localeTools.py:128: raise Exception( On 2017/09/28 10:48:40, tlucas wrote: > On ...
2 years, 4 months ago (2017-09-28 11:34:44 UTC) #12
tlucas
Patch Set 8 * Print actual HTTP Error On 2017/09/28 11:34:44, kzar wrote: > https://codereview.adblockplus.org/29556601/diff/29557586/localeTools.py ...
2 years, 4 months ago (2017-09-28 12:12:36 UTC) #13
kzar
LGTM
2 years, 4 months ago (2017-09-28 12:21:22 UTC) #14
Vasily Kuznetsov
Hi Tristan, A couple more nits (see below). Cheers, Vasily https://codereview.adblockplus.org/29556601/diff/29558616/localeTools.py File localeTools.py (right): https://codereview.adblockplus.org/29556601/diff/29558616/localeTools.py#newcode107 ...
2 years, 4 months ago (2017-09-28 14:35:01 UTC) #15
tlucas
Patch Set 9 * Applied Vasily's comments https://codereview.adblockplus.org/29556601/diff/29558616/localeTools.py File localeTools.py (right): https://codereview.adblockplus.org/29556601/diff/29558616/localeTools.py#newcode107 localeTools.py:107: query = ...
2 years, 4 months ago (2017-09-28 15:53:41 UTC) #16
Vasily Kuznetsov
LGTM
2 years, 4 months ago (2017-09-28 19:24:46 UTC) #17
Sebastian Noack
https://codereview.adblockplus.org/29556601/diff/29556656/localeTools.py File localeTools.py (right): https://codereview.adblockplus.org/29556601/diff/29556656/localeTools.py#newcode106 localeTools.py:106: query = dict(urlparse.parse_qsl(query)) On 2017/09/27 10:51:31, tlucas wrote: > ...
2 years, 4 months ago (2017-09-28 20:25:21 UTC) #18
tlucas
Patch Set 10 * Applied Sebastian's comments * Fixed an error where the query was ...
2 years, 4 months ago (2017-09-28 21:33:59 UTC) #19
Sebastian Noack
https://codereview.adblockplus.org/29556601/diff/29558680/localeTools.py File localeTools.py (right): https://codereview.adblockplus.org/29556601/diff/29558680/localeTools.py#newcode101 localeTools.py:101: return '{}/{}/{}?{}'.format(CROWDIN_AP_URL, On 2017/09/28 21:33:58, tlucas wrote: > as ...
2 years, 4 months ago (2017-09-28 22:18:38 UTC) #20
Sebastian Noack
https://codereview.adblockplus.org/29556601/diff/29558680/localeTools.py File localeTools.py (right): https://codereview.adblockplus.org/29556601/diff/29558680/localeTools.py#newcode354 localeTools.py:354: 'Content-Type: application/octet-stream; charset=utf-8\r\n' On 2017/09/28 22:18:37, Sebastian Noack wrote: ...
2 years, 4 months ago (2017-09-28 22:51:56 UTC) #21
tlucas
Patch Set 11 * removed crowdin_url() * guessing mimetype instead of hardcoding https://codereview.adblockplus.org/29556601/diff/29558680/localeTools.py File localeTools.py ...
2 years, 4 months ago (2017-09-29 09:09:37 UTC) #22
Sebastian Noack
2 years, 4 months ago (2017-09-29 19:20:31 UTC) #23
LGTM
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld 87257f5