Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Discussion of Language Versioning design document. #311

Closed
lrhn opened this issue Apr 11, 2019 · 8 comments
Closed

Discussion of Language Versioning design document. #311

lrhn opened this issue Apr 11, 2019 · 8 comments
Labels
feature Proposed language feature that solves one or more problems

Comments

@lrhn
Copy link
Member

lrhn commented Apr 11, 2019

I have created https://github.com/dart-lang/language/blob/master/accepted/future-releases/language-versioning/language-versioning.md
This is the first draft of the design document for a "language versioning" feature (a solution for issue #94).

Use this issue for comments and discussions about the design document.

@lrhn lrhn added the feature Proposed language feature that solves one or more problems label Apr 11, 2019
@danrubel
Copy link

This spec includes a way to bundle un-packaged files into a package in the .package file, but...

  1. In non package libraries, I see the syntax *=quiver
  2. In manual upgrade example, I see the syntax *:floo

Which syntax is correct?

@bwilkerson
Copy link
Member

We might want to split this out into a separate thread, but we also have a more general question.

The proposal looks like a good way to handle opting into language features at the package level and opting out at the library level. As I understand it, when a new feature is shipped, packages specify a minimum SDK constraint that is at or latter than the first version of the SDK that included the feature in order to opt in to using it.

The question is: how do we work with this in the period of time when we are testing an experimental feature that hasn't yet shipped?

There are two difficulties that I have thought of. First, packages can't specify a minimum SDK that is later than the current SDK; pub produces an error message and quits. Second, we don't yet have a version number to associate with the feature; that won't happen until it ships.

We considered a couple of possibilities. One was to use the enablement of the experiment as a signal, but that doesn't make it easy to test cases where some packages are opted in and others are not. Another was to update pub to allow SDK constraints after the current SDK version, but that reduces our ability to report real errors to real users. We also considered various out-of-band ways to implement something that would only work during this period of time.

In my opinion, the closer the solution is to the way it will work after the experiment is enabled, the better, because we'll be testing what users will experience rather than some temporary path that will be replaced when the feature ships.

This hasn't been an issue with previous experiments because they weren't breaking and hence didn't really need to be opted in to, but I think this will be a much more important question for nnbd and future breaking changes.

Also, I think the way we answer this question might well impact the way we write language_2 tests, so understanding how this works in the non-package library is also useful.

@danrubel
Copy link

Scanner/parser clients other than analyzer turn off scanning of comments for efficiency. Given this spec, they will need to parse at least some comments. It would be useful to narrow the opt-out comment format to maintain as much efficiency as possible.

I propose only allowing comments of the form

// @dart = 2.0

to opt-out a library. Whitespace allowed everywhere except in @dart and in the version number. Doing so would allow us to maintain the efficient scanning/parsing we have today.

If this sounds reasonable, I'll create a pull request with the spec change.

@lrhn
Copy link
Member Author

lrhn commented Apr 11, 2019

@danrubel
Format for default package should be *:packageName. Other lines use : as separator. Good catch.

About comments, the things you are disallowing are:

/* @dart = 2.0 */

or

/*
 * @dart = 2.0 
 */

(or even /// @dart = 2.0?)
and spaces between @ and dart, right?

@danrubel
Copy link

@lrhn Correct... only allow // @dart = <major>.<minor>

We can enhance / extend later if necessary, but I believe that's all we need.

@danrubel
Copy link

PR #316 updates the spec with this simplification.

@bwilkerson
Copy link
Member

I created a new issue to discuss a new topic: #318. If you'd prefer I can copy the text into this issue and close the other.

@lrhn
Copy link
Member Author

lrhn commented Jun 25, 2020

Language versioning has landed.

@lrhn lrhn closed this as completed Jun 25, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature Proposed language feature that solves one or more problems
Projects
None yet
Development

No branches or pull requests

3 participants