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

Issue 29968558: Issue 7037 - [XTM Integration] Make REST API url customizable

Can't Edit
Can't Publish+Mail
Start Review
Created:
5 months ago by Tudor Avram
Modified:
4 months, 2 weeks ago
Reviewers:
Vasily Kuznetsov
CC:
wspee
Visibility:
Public.

Description

Issue 7037 - [XTM Integration] Make REST API url customizable

Patch Set 1 #

Total comments: 1
Unified diffs Side-by-side diffs Delta from patch set Stats (+138 lines, -34 lines) Patch
M cms/translations/xtm/cli.py View 3 chunks +10 lines, -7 lines 1 comment Download
M cms/translations/xtm/constants.py View 3 chunks +4 lines, -0 lines 0 comments Download
M cms/translations/xtm/utils.py View 2 chunks +39 lines, -0 lines 0 comments Download
M cms/translations/xtm/xtm_api.py View 11 chunks +27 lines, -12 lines 0 comments Download
M tests/test_xtm_api.py View 8 chunks +10 lines, -8 lines 0 comments Download
M tests/test_xtm_translate.py View 3 chunks +3 lines, -3 lines 0 comments Download
M tests/test_xtm_translations_utils.py View 6 chunks +24 lines, -4 lines 0 comments Download
M tests/utils.py View 2 chunks +8 lines, -0 lines 0 comments Download
M tests/xtm_conftest.py View 1 chunk +13 lines, -0 lines 0 comments Download

Messages

Total messages: 2
Tudor Avram
Hi Vasily and Winsley, I changed the implementation of the XTM integration to support customisation ...
5 months ago (2018-12-27 14:25:36 UTC) #1
Vasily Kuznetsov
4 months, 2 weeks ago (2019-01-10 17:45:23 UTC) #2
Hi Tudor,

This looks pretty good. I just have one question (see below).

Cheers,
Vasily

https://codereview.adblockplus.org/29968558/diff/29968559/cms/translations/xt...
File cms/translations/xtm/cli.py (right):

https://codereview.adblockplus.org/29968558/diff/29968559/cms/translations/xt...
cms/translations/xtm/cli.py:36: token = read_token()
Wouldn't it be better to keep this inside of try/except so that the exception
would be reported nicely?
Sign in to reply to this message.

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