components/viz/host/gpu/DEPS - Issue 2939953004: viz: Move some code into //components/viz/common.

Side by Side Diff

Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.

Keyboard Shortcuts

	File
u :	up to issue
j / k :	jump to file after / before current file
J / K :	jump to next file with a comment after / before current file
	Side-by-side diff
i :	toggle intra-line diffs
e :	expand all comments
c :	collapse all comments
s :	toggle showing all comments
n / p :	next / previous diff chunk or comment
N / P :	next / previous comment
<Up> / <Down> :	next / previous line

	Issue
u :	up to list of issues
j / k :	jump to patch after / before current patch
o / <Enter> :	open current patch in side-by-side view
i :	open current patch in unified diff view

	Issue List
j / k :	jump to issue after / before current issue
o / <Enter> :	open current issue

Side by Side Diff: components/viz/host/gpu/DEPS

Issue 2939953004: viz: Move some code into //components/viz/common. (Closed)

Patch Set: . Created 3 years, 6 months ago

Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.

Jump to:

View unified diff | Download patch

OLD	NEW
(Empty)
	1 include_rules = [

	2 "+services/ui/gpu/interfaces",
	danakj 2017/06/14 21:34:06 Is it weird to dep from components to services? Ca Is it weird to dep from components to services? Can you convince me this isn't weird, and we don't need to do dependency injection instead? Fady Samuel 2017/06/14 21:35:23 I think the interface should just move here... Show quoted text On 2017/06/14 21:34:06, danakj wrote: > Is it weird to dep from components to services? Can you convince me this isn't > weird, and we don't need to do dependency injection instead? I think the interface should just move here... sadrul 2017/06/14 21:40:59 My opinion is that interfaces should remain under Show quoted text On 2017/06/14 21:35:23, Fady Samuel wrote: > On 2017/06/14 21:34:06, danakj wrote: > > Is it weird to dep from components to services? Can you convince me this isn't > > weird, and we don't need to do dependency injection instead? > > I think the interface should just move here... My opinion is that interfaces should remain under //services/. One of the reasons we are moving code out of //services/ui/* into //components/viz is to avoid introducing dependency on interface-implementation code in //services/ from //content. So it should be OK to leave just the interfaces in their place under //services. This does not drag in additional dependency on impl code in //services/, so this should be acceptable. danakj 2017/06/14 21:52:08 Ok thanks, so the rule would be .../interfaces is Show quoted text On 2017/06/14 21:40:59, sadrul wrote: > On 2017/06/14 21:35:23, Fady Samuel wrote: > > On 2017/06/14 21:34:06, danakj wrote: > > > Is it weird to dep from components to services? Can you convince me this > isn't > > > weird, and we don't need to do dependency injection instead? > > > > I think the interface should just move here... > > My opinion is that interfaces should remain under //services/. One of the > reasons we are moving code out of //services/ui/* into //components/viz is to > avoid introducing dependency on interface-implementation code in //services/ > from //content. So it should be OK to leave just the interfaces in their place > under //services. This does not drag in additional dependency on impl code in > //services/, so this should be acceptable. Ok thanks, so the rule would be .../interfaces is okay, but nothing else. A comment here explaining this rule would be appreciated, and maybe up at a higher level in c/viz/README.md so you can point me at that in the future :) One more Q to verify, but what you said there makes sense to me: Do the interfaces in this directory always exist? Would there be cases where we want to use this component but not use these services interfaces? In content? In tests? (Assume tests are happy making mojo interfaces in general.)
	3 ]

OLD	NEW

« no previous file with comments | « components/viz/host/gpu/BUILD.gn ('k') | components/viz/host/gpu/server_gpu_memory_buffer_manager.h » ('j') | services/ui/gpu/BUILD.gn » ('J')