Issue 1457383005: Require an explicit ServiceName annotation for interfaces in Java

Issue 1457383005: Require an explicit ServiceName annotation for interfaces in Java (Closed)

Created:
5 years, 1 month ago by qsr

Modified:
5 years, 1 month ago

Reviewers:
ppi

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.

More Reviews

Description

Require an explicit ServiceName annotation for interfaces in Java The names added match the current implicit service names. BUG=https://github.com/domokit/mojo/issues/498 R=ppi@chromium.org Committed: https://chromium.googlesource.com/external/mojo/+/38b3d0f01b060a0ea9bbe15131840f63a72fb6f9

Patch Set 1 #

Total comments: 4

Patch Set 2 : Follow review #

Created: 5 years, 1 month ago

Download [raw] [tar.bz2]

	Unified diffs	Side-by-side diffs	Delta from patch set	Stats (+30 lines, -11 lines)			Patch
M	mojo/public/interfaces/bindings/tests/sample_factory.mojom	View		1 chunk	+1 line, -0 lines	0 comments	Download
M	mojo/public/java/application/src/org/chromium/mojo/application/ShellHelper.java	View		1 chunk	+2 lines, -2 lines	0 comments	Download
M	mojo/public/java/bindings/src/org/chromium/mojo/bindings/Interface.java	View	1	2 chunks	+11 lines, -6 lines	0 comments	Download
M	mojo/public/tools/bindings/generators/java_templates/interface_definition.tmpl	View		1 chunk	+4 lines, -2 lines	0 comments	Download
M	mojo/services/camera/interfaces/camera.mojom	View		2 chunks	+2 lines, -0 lines	0 comments	Download
M	mojo/services/input/interfaces/input.mojom	View		1 chunk	+1 line, -0 lines	0 comments	Download
M	mojo/services/location/interfaces/location_service.mojom	View		1 chunk	+1 line, -0 lines	0 comments	Download
M	mojo/services/sensors/interfaces/sensors.mojom	View		1 chunk	+1 line, -0 lines	0 comments	Download
M	mojo/services/sharing/interfaces/sharing.mojom	View		1 chunk	+1 line, -0 lines	0 comments	Download
M	mojo/services/speech_recognizer/interfaces/speech_recognizer.mojom	View		2 chunks	+2 lines, -1 line	0 comments	Download
M	mojo/services/vsync/interfaces/vsync.mojom	View		1 chunk	+1 line, -0 lines	0 comments	Download
M	services/intent_receiver/intent_receiver.mojom	View		1 chunk	+1 line, -0 lines	0 comments	Download
M	services/nfc_message_sink/nfc_message_sink.mojom	View		1 chunk	+1 line, -0 lines	0 comments	Download
M	services/sharing_sink/sharing_sink.mojom	View		1 chunk	+1 line, -0 lines	0 comments	Download

Messages

Total messages: 4 (0 generated)

Expand Messages | Collapse Messages

ppi

lgtm % nits https://codereview.chromium.org/1457383005/diff/1/mojo/public/java/bindings/src/org/chromium/mojo/bindings/Interface.java File mojo/public/java/bindings/src/org/chromium/mojo/bindings/Interface.java (right): https://codereview.chromium.org/1457383005/diff/1/mojo/public/java/bindings/src/org/chromium/mojo/bindings/Interface.java#newcode439 mojo/public/java/bindings/src/org/chromium/mojo/bindings/Interface.java:439: * The |Manager| object for interfaces ...

5 years, 1 month ago (2015-11-20 09:51:11 UTC) #2

qsr

Committed patchset #2 (id:20001) manually as 38b3d0f01b060a0ea9bbe15131840f63a72fb6f9 (presubmit successful).

5 years, 1 month ago (2015-11-20 09:53:02 UTC) #3

qsr

5 years, 1 month ago (2015-11-20 09:53:04 UTC) #4

Message was sent while issue was closed.

https://codereview.chromium.org/1457383005/diff/1/mojo/public/java/bindings/s...
File mojo/public/java/bindings/src/org/chromium/mojo/bindings/Interface.java
(right):

https://codereview.chromium.org/1457383005/diff/1/mojo/public/java/bindings/s...
mojo/public/java/bindings/src/org/chromium/mojo/bindings/Interface.java:439: *
The |Manager| object for interfaces having a name in the bindings.
On 2015/11/20 09:51:11, ppi wrote:
> s/having a name/having an associated service name/ (because interfaces always
> have names)

Done.

https://codereview.chromium.org/1457383005/diff/1/mojo/public/java/bindings/s...
mojo/public/java/bindings/src/org/chromium/mojo/bindings/Interface.java:443: *
Returns the name of the interface. This is an opaque (but human readable)
identifier used
On 2015/11/20 09:51:11, ppi wrote:
> Returns the name of the service? AFAIU it is actually different from "the name
> of the interface".

Done.

Expand Messages | Collapse Messages