Issue 10829181: Redesign TabBase to get rid of mandatory inheritance of dialog element interfaces

Issue 10829181: Redesign TabBase to get rid of mandatory inheritance of dialog element interfaces (Closed)

Created:
8 years, 4 months ago by Peter Rybin

Modified:
8 years, 4 months ago

Reviewers:
apavlov

CC:
chromedevtools-codereview_googlegroups.com

Base URL:
https://chromedevtools.googlecode.com/svn/trunk

Visibility:
Public.

More Reviews

Description

Redesign TabBase to get rid of mandatory inheritance of dialog element interfaces Committed: https://code.google.com/p/chromedevtools/source/detail?r=1036

Patch Set 2 : fcr #

Created: 8 years, 4 months ago

Download [raw] [tar.bz2]

	Unified diffs	Side-by-side diffs	Delta from patch set	Stats (+128 lines, -48 lines)			Patch
M	plugins/org.chromium.debug.ui/src/org/chromium/debug/ui/launcher/ChromiumRemoteTab.java	View		5 chunks	+4 lines, -9 lines	0 comments	Download
M	plugins/org.chromium.debug.ui/src/org/chromium/debug/ui/launcher/ScriptMappingTab.java	View		2 chunks	+3 lines, -3 lines	0 comments	Download
M	plugins/org.chromium.debug.ui/src/org/chromium/debug/ui/launcher/TabBase.java	View	1	3 chunks	+98 lines, -10 lines	0 comments	Download
M	plugins/org.chromium.debug.ui/src/org/chromium/debug/ui/launcher/WipRemoteTab.java	View	1	4 chunks	+23 lines, -26 lines	0 comments	Download

Messages

Total messages: 3 (0 generated)

Expand Messages | Collapse Messages

apavlov

LGTM with comments http://codereview.chromium.org/10829181/diff/1/plugins/org.chromium.debug.ui/src/org/chromium/debug/ui/launcher/TabBase.java File plugins/org.chromium.debug.ui/src/org/chromium/debug/ui/launcher/TabBase.java (right): http://codereview.chromium.org/10829181/diff/1/plugins/org.chromium.debug.ui/src/org/chromium/debug/ui/launcher/TabBase.java#newcode391 plugins/org.chromium.debug.ui/src/org/chromium/debug/ui/launcher/TabBase.java:391: * Incapsulate tab field list together ...

8 years, 4 months ago (2012-08-06 08:19:06 UTC) #2

Peter Rybin

8 years, 4 months ago (2012-08-06 22:40:35 UTC) #3

http://codereview.chromium.org/10829181/diff/1/plugins/org.chromium.debug.ui/...
File
plugins/org.chromium.debug.ui/src/org/chromium/debug/ui/launcher/TabBase.java
(right):

http://codereview.chromium.org/10829181/diff/1/plugins/org.chromium.debug.ui/...
plugins/org.chromium.debug.ui/src/org/chromium/debug/ui/launcher/TabBase.java:391:
* Incapsulate tab field list together with a way of accessing them. This
includes adapting
On 2012/08/06 08:19:06, apavlov wrote:
> Encapsulate

Done.

http://codereview.chromium.org/10829181/diff/1/plugins/org.chromium.debug.ui/...
plugins/org.chromium.debug.ui/src/org/chromium/debug/ui/launcher/TabBase.java:437:
* @param elementsAdapter converts one dialog elements structure type to another
On 2012/08/06 08:19:06, apavlov wrote:
> what are "one" and "another" in this context? This should be explained in the
> javadoc

Done.

http://codereview.chromium.org/10829181/diff/1/plugins/org.chromium.debug.ui/...
File
plugins/org.chromium.debug.ui/src/org/chromium/debug/ui/launcher/WipRemoteTab.java
(right):

http://codereview.chromium.org/10829181/diff/1/plugins/org.chromium.debug.ui/...
plugins/org.chromium.debug.ui/src/org/chromium/debug/ui/launcher/WipRemoteTab.java:96:
TabField<String, String, WipTabElements, Params> backendChooser = new
TabField<String, String, WipTabElements, Params>(
On 2012/08/06 08:19:06, apavlov wrote:
> line way too long

Done.

http://codereview.chromium.org/10829181/diff/1/plugins/org.chromium.debug.ui/...
plugins/org.chromium.debug.ui/src/org/chromium/debug/ui/launcher/WipRemoteTab.java:118:
List<TabFieldList<? super WipTabElements, ? super Params>> list  = new
ArrayList<TabFieldList<? super WipTabElements, ? super Params>>(2);
On 2012/08/06 08:19:06, apavlov wrote:
> line way too long

Done.

Expand Messages | Collapse Messages