pkg: analysis aided cleanup

pkg: analysis aided cleanup

Removed a lot of warnings and hints when opening many pkg projects in the editor

R=gram@google.com

Committed: https://code.google.com/p/dart/source/detail?r=25831

[gram, 2013-08-05 23:23:12]

For the most part lgtm. I assume you have run the tests for all these? I'd like
Bob and/or Nathan to opine on whether packages should take a top-level
dependency on unittest or whether they should have a second pubspec in their
test directories.

https://codereview.chromium.org/22284003/diff/1/pkg/http_server/pubspec.yaml
File pkg/http_server/pubspec.yaml (right):

https://codereview.chromium.org/22284003/diff/1/pkg/http_server/pubspec.yaml#...
pkg/http_server/pubspec.yaml:8: unittest: any
See my comment elsewhere. Bob and/or Nathan should weigh in here.

https://codereview.chromium.org/22284003/diff/1/pkg/path/pubspec.yaml
File pkg/path/pubspec.yaml (right):

https://codereview.chromium.org/22284003/diff/1/pkg/path/pubspec.yaml#newcode10
pkg/path/pubspec.yaml:10: unittest: any
Do we need this unittest dependency here?

I'm not sure what the right convention should be. Personally I favor putting a
pubspec with a unittest dependency in the test directory, and not having it in
the main package directory; that means you can open up the test directory in the
editor and run the tests after a pub install, and if you're just using a package
and not testing it you don't need the dependency here.

Bob, Nathan, do you have an opinion here? Should unittest go in as a dependency
of all pub packages with tests in their top-level pubspecs?

https://codereview.chromium.org/22284003/diff/1/pkg/scheduled_test/lib/src/de...
File pkg/scheduled_test/lib/src/descriptor/file_descriptor.dart (right):

https://codereview.chromium.org/22284003/diff/1/pkg/scheduled_test/lib/src/de...
pkg/scheduled_test/lib/src/descriptor/file_descriptor.dart:12: import
'package:path/path.dart' as pathos;
Is there a conflict with path? Otherwise we should just use it; the 'pathos'
name is deprecated AFAIK.

[nweiz, 2013-08-06 00:06:08]

On 2013/08/05 23:23:12, gram wrote:
> For the most part lgtm. I assume you have run the tests for all these? I'd
like
> Bob and/or Nathan to opine on whether packages should take a top-level
> dependency on unittest or whether they should have a second pubspec in their
> test directories.
> 
> https://codereview.chromium.org/22284003/diff/1/pkg/http_server/pubspec.yaml
> File pkg/http_server/pubspec.yaml (right):
> 
>
https://codereview.chromium.org/22284003/diff/1/pkg/http_server/pubspec.yaml#...
> pkg/http_server/pubspec.yaml:8: unittest: any
> See my comment elsewhere. Bob and/or Nathan should weigh in here.
> 
> https://codereview.chromium.org/22284003/diff/1/pkg/path/pubspec.yaml
> File pkg/path/pubspec.yaml (right):
> 
>
https://codereview.chromium.org/22284003/diff/1/pkg/path/pubspec.yaml#newcode10
> pkg/path/pubspec.yaml:10: unittest: any
> Do we need this unittest dependency here?
> 
> I'm not sure what the right convention should be. Personally I favor putting a
> pubspec with a unittest dependency in the test directory, and not having it in
> the main package directory; that means you can open up the test directory in
the
> editor and run the tests after a pub install, and if you're just using a
package
> and not testing it you don't need the dependency here.
> 
> Bob, Nathan, do you have an opinion here? Should unittest go in as a
dependency
> of all pub packages with tests in their top-level pubspecs?
> 
>
https://codereview.chromium.org/22284003/diff/1/pkg/scheduled_test/lib/src/de...
> File pkg/scheduled_test/lib/src/descriptor/file_descriptor.dart (right):
> 
>
https://codereview.chromium.org/22284003/diff/1/pkg/scheduled_test/lib/src/de...
> pkg/scheduled_test/lib/src/descriptor/file_descriptor.dart:12: import
> 'package:path/path.dart' as pathos;
> Is there a conflict with path? Otherwise we should just use it; the 'pathos'
> name is deprecated AFAIK.

All dependencies, including those only used for tests, should be declared in the
top-level directory. This is what dev dependencies are for. Pub has no support
for nested pubspecs; it may work to use them in some circumstances, but having
two different sources for what should be in a "packages" directory is a bad
idea.

[kevmoo-old, 2013-08-06 01:27:16]

Addressed comments. Several more fixes. PTAL

https://codereview.chromium.org/22284003/diff/1/pkg/scheduled_test/lib/src/de...
File pkg/scheduled_test/lib/src/descriptor/file_descriptor.dart (right):

https://codereview.chromium.org/22284003/diff/1/pkg/scheduled_test/lib/src/de...
pkg/scheduled_test/lib/src/descriptor/file_descriptor.dart:12: import
'package:path/path.dart' as pathos;
On 2013/08/05 23:23:12, gram wrote:
> Is there a conflict with path? Otherwise we should just use it; the 'pathos'
> name is deprecated AFAIK.

The editor shows some really weird errors when I leave it as 'path'. But
everything runs fine.

https://codereview.chromium.org/22284003/diff/1/pkg/scheduled_test/lib/src/de...
pkg/scheduled_test/lib/src/descriptor/file_descriptor.dart:12: import
'package:path/path.dart' as pathos;
On 2013/08/05 23:23:12, gram wrote:
> Is there a conflict with path? Otherwise we should just use it; the 'pathos'
> name is deprecated AFAIK.

Done.

[gram, 2013-08-06 19:51:41]

lgtm

[kevmoo-old, 2013-08-06 20:44:25]

Committed patchset #2 manually as r25831 (presubmit successful).