Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(423)

Issue 1508613003: Require an explicit ServiceName annotation for interfaces in Python. (Closed)

Created:
5 years ago by azani
Modified:
5 years ago
Reviewers:
qsr
CC:
mojo-reviews_chromium.org, gregsimon, qsr+mojo_chromium.org, viettrungluu+watch_chromium.org, abarth-chromium, Aaron Boodman, darin (slow to review), ben+mojo_chromium.org, yzshen+mojopublicwatch_chromium.org
Base URL:
https://github.com/domokit/mojo.git@master
Target Ref:
refs/heads/master
Project:
mojo
Visibility:
Public.

Description

Require an explicit ServiceName annotation for interfaces in Python. BUG= #498 R=qsr@chromium.org Committed: https://chromium.googlesource.com/external/mojo/+/dccf9b1b71d8f3d7419b35c70a27140f57d481a6

Patch Set 1 #

Patch Set 2 : #

Patch Set 3 : #

Total comments: 1

Patch Set 4 : #

Patch Set 5 : #

Patch Set 6 : #

Unified diffs Side-by-side diffs Delta from patch set Stats (+17 lines, -6 lines) Patch
M mojo/public/python/mojo_application/application_impl.py View 1 2 3 4 2 chunks +6 lines, -1 line 0 comments Download
M mojo/public/python/mojo_application/service_provider_impl.py View 1 2 3 1 chunk +7 lines, -2 lines 0 comments Download
M mojo/public/python/mojo_bindings/interface_reflection.py View 1 2 3 2 chunks +3 lines, -2 lines 0 comments Download
M mojo/public/tools/bindings/generators/python_templates/module.py.tmpl View 1 chunk +1 line, -1 line 0 comments Download

Messages

Total messages: 8 (2 generated)
azani
ptal
5 years ago (2015-12-08 16:48:14 UTC) #2
qsr
That seems too small. For example look at interface_reflection.py, line 150. self.name will be None. ...
5 years ago (2015-12-09 12:39:50 UTC) #3
azani
Thanks for the feedback. ptal With this change, I'm slightly modifying the behavior of the ...
5 years ago (2015-12-09 21:09:11 UTC) #4
qsr
On 2015/12/09 21:09:11, azani wrote: > Thanks for the feedback. ptal > > With this ...
5 years ago (2015-12-10 15:13:39 UTC) #5
qsr
LGTM with nit. https://codereview.chromium.org/1508613003/diff/40001/mojo/public/python/mojo_application/application_impl.py File mojo/public/python/mojo_application/application_impl.py (right): https://codereview.chromium.org/1508613003/diff/40001/mojo/public/python/mojo_application/application_impl.py#newcode51 mojo/public/python/mojo_application/application_impl.py:51: if not service_class.service_name: I think service_name ...
5 years ago (2015-12-10 15:13:54 UTC) #6
azani
5 years ago (2015-12-10 23:17:52 UTC) #8
Message was sent while issue was closed.
Committed patchset #6 (id:100001) manually as
dccf9b1b71d8f3d7419b35c70a27140f57d481a6 (presubmit successful).

Powered by Google App Engine
This is Rietveld 408576698