move cli main into lib/
#1114
move cli main into lib/
#1114
Conversation
|
|
||
| if (customPackageRoot != null && packageConfigFile != null) { | ||
| errorSink.write("Cannot specify both '--package-root' and '--packages'."); | ||
| exitCode = unableToProcessExitCode; |
There was a problem hiding this comment.
pro tip: return the exit code from this method – set it in bin. Makes testing MUCH easier.
There was a problem hiding this comment.
Great point. That said, I think these cases are pretty well covered in our integration tests. Are you making a general point or do you see cases we're missing?
| // ignore: avoid_catches_without_on_clauses | ||
| } catch (err, stack) { | ||
| errorSink.writeln('''An error occurred while linting | ||
| Please report it at: github.com/dart-lang/linter/issues |
There was a problem hiding this comment.
Move this catch to bin – makes testing easier
There was a problem hiding this comment.
...make sure to set the exit code to an error!
|
General point – I find it much easier to reason about code that way.
…On Tue, Aug 7, 2018 at 12:16 PM Phil Quitslund ***@***.***> wrote:
***@***.**** commented on this pull request.
------------------------------
In lib/src/cli.dart
<#1114 (comment)>:
> + lintOptions.dartSdkPath = customSdk;
+ }
+
+ var strongMode = options['strong'];
+ if (strongMode != null) lintOptions.strongMode = strongMode;
+
+ var customPackageRoot = options['package-root'];
+ if (customPackageRoot != null) {
+ lintOptions.packageRootPath = customPackageRoot;
+ }
+
+ var packageConfigFile = options['packages'];
+
+ if (customPackageRoot != null && packageConfigFile != null) {
+ errorSink.write("Cannot specify both '--package-root' and '--packages'.");
+ exitCode = unableToProcessExitCode;
Great point. That said, I think these cases are pretty well covered in our
integration tests. Are you making a general point or do you see cases we're
missing?
—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub
<#1114 (comment)>, or mute
the thread
<https://github.com/notifications/unsubscribe-auth/AABCil2NM5gH6nrREv5c5duLxw3d-n36ks5uOeeVgaJpZM4VylK_>
.
|
|
Thanks @kevmoo! I think our integration tests cover the exit code cases pretty well as is but I'll look at our updated coverage results after this lands and see if a refactor is in order. As for the general point, it's a good one. In the short to medium term I'm hoping this code will go away altogether (see dart-lang/sdk#57427). If it doesn't maybe we can rejigger it a bit. Cheers! |
Fixes: dart-lang/sdk#57758.
/cc @bwilkerson @grouma @a14n