Move some console related code from debugger and inspector screens to shared/console. #4991
Conversation
polina-c
changed the title
polina-c
changed the title
polina-c
changed the title
polina-c
requested review from
a team,
CoderDake,
bkonyi,
elliette and
kenzieschmoll
as code owners
polina-c
changed the title
| import 'dart:developer'; | ||
|
|
||
| import 'package:flutter/cupertino.dart'; | ||
| import 'package:flutter/foundation.dart'; |
There was a problem hiding this comment.
This has a lot of non-console stuff in it. I would suggest either leaving it in the inspector and accepting slightly tangled imports or move it to a shared directory. Seems odd to have this core inspector logic under the console rather than inspector or shared.
| @@ -255,7 +250,7 @@ class DiagnosticsNodeDescription extends StatelessWidget { | |||
| final defaultStyle = DefaultTextStyle.of(context).style; | |||
| final baseStyle = style ?? defaultStyle; | |||
| TextStyle textStyle = baseStyle.merge( | |||
| inspector_text_styles.textStyleForLevel( | |||
There was a problem hiding this comment.
this seems a bit inverted. these are much more inspector specific styles than console styles. If you want a neutral name, these could be diagnostics_text_styles.
| @@ -1,20 +1,19 @@ | |||
| // Copyright 2018 The Chromium Authors. All rights reserved. | |||
| // Use of this source code is governed by a BSD-style license that can be | |||
There was a problem hiding this comment.
This also isn't a console thing. If you want to move it, it should go in shared not console.
|
|
||
| import '../../theme.dart'; | ||
|
|
||
| // Enum-like static classes are ok. |
There was a problem hiding this comment.
why not continue the previous pattern of top level members imported with a prefix? In general if we were to use one pattern I would think we would migrate to that pattern not to this pattern that violates the lints we have enabled.
There was a problem hiding this comment.
Discussed offline and agreed enum-like static classes are more convenient than top level members.
jacob314
left a comment
jacob314
left a comment
There was a problem hiding this comment.
LGTM as long as there is a cleanup CL after the 3 chained CLs are landed moving things to a common dir.
2 participants
RELEASE_NOTE_EXCEPTION=[no functional changes]