Refer to library by URL. #138
Conversation
| @@ -4,7 +4,7 @@ author: Dart Team <[email protected]> | |||
| description: Automatic sourcecode generation for Dart | |||
| homepage: https://github.com/dart-lang/source_gen | |||
| environment: | |||
| sdk: '>=1.12.0 <2.0.0' | |||
| sdk: '>=1.23.0-dev.11.0 <2.0.0' | |||
There was a problem hiding this comment.
Is this about the SDK min value ... or pkg/analyzer min version?
There was a problem hiding this comment.
Probably need both? SDK for supporting the output in tools/runtimes, analyzer for supporting parsing this file in later codegen steps etc?
There was a problem hiding this comment.
Strictly speaking, this package doesn't need to align w/ the SDK version. Just the analyzer version.
One could generate code with SDK X that doesn't run in SDK X.
...right?
There was a problem hiding this comment.
Yes, we could generate code that doesn't run. I guess if we do that we would instead need to bump the source_gen major version? Maybe we should do that anyway?
--> no new SDK requirement
--> version bump to 0.6
What do you think?
There was a problem hiding this comment.
Friendly ping. Any ideas how we should ship this? Thanks.
There was a problem hiding this comment.
I think we should bump the min sdk version but not do a major version bump in this package (unless there is a breaking change I am missing).
There isn't much of a point in generating code that can't run in your current sdk imo, it would just lead to confusion.
There was a problem hiding this comment.
Ack, kept it with just the SDK version bump.
test/builder_test.dart
Outdated
| @@ -160,7 +160,8 @@ class Customer { } | |||
|
|
|||
| const _testGenPartContent = r'''// GENERATED CODE - DO NOT MODIFY BY HAND | |||
|
|
|||
| part of test_lib; | |||
| part of 'test_lib.dart'; | |||
|
|
|||
There was a problem hiding this comment.
where is this extra new-line coming from?
There was a problem hiding this comment.
:/ don't know ... it may be some weirdness with dartfmt.
test/builder_test.dart
Outdated
| @@ -160,7 +160,8 @@ class Customer { } | |||
|
|
|||
| const _testGenPartContent = r'''// GENERATED CODE - DO NOT MODIFY BY HAND | |||
|
|
|||
| part of test_lib; | |||
| part of 'test_lib.dart'; | |||
There was a problem hiding this comment.
Also: We could remove the library name from test_lib.dart, right?
There was a problem hiding this comment.
Yes. If we want to push this before the analyzer is fixed, we'll also want to generate the appropriate '// ignore: '. WDYT?
There was a problem hiding this comment.
Lets just generate it with the // ignore: for now.
There was a problem hiding this comment.
Added the "ignore".
|
There seems to be an issue with dartfmt; it appears to be refusing to format the output altogether. When I run dartfmt locally, it actually deletes the URL from 'part of'. So I think this CL will have to wait for a dartfmt fix. I'll see if there's an issue open. |
|
@davidmorgan Ping. This was fixed as of May 9th 👍 |
|
@davidmorgan I'm proposing to supersede this with #179. |
This will let people get rid of 'library' statements.
Really looking forward to this change -- any plans for when we can make it? Do we need to wait for 1.23 to be in the stable channel?
I checked built_value's end to end tests with 1.23.0-dev.11.0 and it works fine. The analyzed currently complains, but we don't necessarily need to wait for that -- we can silence it with "// ignore: non_identifier_library_name".
Thanks!