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

Remove uses of deprecated constants from packages used by flutter and internal code #32749

Closed
leafpetersen opened this issue Apr 3, 2018 · 33 comments
Assignees
Labels
area-core-library SDK core library issues (core, async, ...); use area-vm or area-web for platform specific libraries.
Milestone

Comments

@leafpetersen
Copy link
Member

Identify and clean up any packages that are used by flutter or by internal code and which still use the deprecated CONSTANTS. Main issue here: #31813 .

@leafpetersen leafpetersen added the area-core-library SDK core library issues (core, async, ...); use area-vm or area-web for platform specific libraries. label Apr 3, 2018
@leafpetersen leafpetersen added this to the Dart2 Beta 3 milestone Apr 3, 2018
@leafpetersen
Copy link
Member Author

@kevmoo I believe you looked at packages to identify ones that would need clean up. Do you still have this data? Can you provide this so that we can cross reference against packages used in flutter and internally?

@kevmoo
Copy link
Member

kevmoo commented Apr 3, 2018 via email

@dgrove
Copy link
Contributor

dgrove commented Apr 6, 2018

What's the next step here? Is there any further work required for beta 3?

@kevmoo
Copy link
Member

kevmoo commented Apr 6, 2018

We have 41 packages that use at least one of these constants. Some of them are used by Flutter. We need to get them updated and published...

Not sure where @nex3 is with https://github.com/dart-lang/dart2_constant

@nex3
Copy link
Member

nex3 commented Apr 6, 2018

Not sure where @nex3 is with https://github.com/dart-lang/dart2_constant

It's released and announced.

@leafpetersen
Copy link
Member Author

leafpetersen commented Apr 6, 2018

It's not clear to me that there is any reason that we need to have these updated and published by Beta-3, since updating the packages is non-breaking to users of the packages (including flutter). Is there something I'm missing? Certainly doesn't hurt to get them done ASAP though.

The intersection of @kvemoo's list of packages from the top 200 that need fixing and packages depended on by flutter is enclosed below. There may be other flutter packages that need fixing that aren't in the top 200 though, so we need to go through and verify at some point.

These packages are used by flutter and need fixing (there may be others):

EDITED: added packages used in sdk that currently use SCREAMING CAPS (at last the version in DEPS)

  • analyzer
  • async
  • barback
  • coverage
  • crypto
  • csslib
  • dart2js_info
  • dart_style
  • html
  • http
  • http_io
  • http_parser
  • http_retry
  • http_throttle
  • image
  • intl
  • intl_translation
  • isolate
  • json_rpc_2
  • json_schema
  • markdown
  • matcher
  • mime
  • mockito
  • mustache4dart
  • oauth2
  • package_resolver
  • pool
  • process
  • protobuf
  • pub
  • quiver
  • resource
  • shelf
  • shelf_static
  • shelf_web_socket
  • source_maps
  • stream_channel
  • test
  • test_process
  • usage
  • vector_math
  • yaml

@devoncarew
Copy link
Member

What is the sdk lower bounds where the constants were introduced (or, are all available)?

@devoncarew
Copy link
Member

I see some packages using: sdk: '>=2.0.0-dev.30 <2.0.0'.

@lrhn
Copy link
Member

lrhn commented Apr 10, 2018

I believe the lower-case constants were released in 2.0.0-dev.17.0

I'm already working on the packages that are used by the SDK, but let's see if I can make proper pull requests for them.

@leafpetersen leafpetersen modified the milestones: Dart2 Beta 3, Dart2 Beta 4 Apr 11, 2018
@leafpetersen
Copy link
Member Author

Added the list of packages used by the sdk to the TODO list above.

@dgrove
Copy link
Contributor

dgrove commented Apr 21, 2018

@lrhn any idea when the changes for these packages can land?

@JekCharlsonYu JekCharlsonYu assigned lrhn and unassigned kevmoo Apr 24, 2018
@dgrove
Copy link
Contributor

dgrove commented Apr 28, 2018

@lrhn any updates?

@lrhn
Copy link
Member

lrhn commented May 15, 2018

I have pull requests out for all the packages in the above list that need it.
Some are entirely third-party, and I don't know how soon those will be handled. We do not control publishing of those (e.g., mustache4dart, which has a dart2 branch, but no releases of it).
Some packages are internal, and some of those are blocked on review (due to review vacation).
Some have already landed.

It's unclear what our strategy for barback is. It's not planned to be upgraded to Dart 2, so we should remove it from the SDK ASAP since it has no future.

The sooner we make the SDK Dart 2 only, the sooner such issues will be surfaced.

@leafpetersen
Copy link
Member Author

@grouma what's the story on barback?

@grouma
Copy link
Member

grouma commented May 15, 2018

It's been removed entirely from pub and is no longer supported: 94f45c8

@leafpetersen
Copy link
Member Author

So can we stop pulling barback into the SDK now?

@kevmoo
Copy link
Member

kevmoo commented May 15, 2018 via email

@leafpetersen
Copy link
Member Author

@dgrove
Copy link
Contributor

dgrove commented Jun 19, 2018

Any updates here?

@kevmoo
Copy link
Member

kevmoo commented Jun 20, 2018

Looks like we still have watcher, markdown, and source_maps not updated and published – of those in the SDK

screen shot 2018-06-19 at 5 22 54 pm

@dgrove
Copy link
Contributor

dgrove commented Jun 22, 2018

/cc @nex3 @sigmundch @munificent - can you publish watcher/source_maps/markdown with new constants?

@dgrove
Copy link
Contributor

dgrove commented Jul 10, 2018

watcher was published 25 June 2018, with new constants.
markdown was published 2 July 2018 - not sure if this has the updated constants. @munificent / @srawlins - do you know?

@munificent
Copy link
Member

I haven't looked into markdown. If @srawlins doesn't reply, I'll take a look.

@srawlins
Copy link
Member

I'll do markdown tonight.

@srawlins
Copy link
Member

Oh, Leaf did it.

@vsmenon
Copy link
Member

vsmenon commented Jul 25, 2018

Any updates on this? Do we have any further changes needed on packages hosted in the sdk repo?

@kevmoo
Copy link
Member

kevmoo commented Jul 25, 2018

Here's our packages. googleapis is important for the Flutter folks – I think that's the only one we should worry about...

screen shot 2018-07-25 at 8 14 36 am

@leafpetersen
Copy link
Member Author

googleapis is not in the transitive closure of the DEPS of the flutter repo, why do you say it's important for them?

The only thing that I know of that is required for flutter is json_schema. I have a PR here that needs some love: Workiva/json_schema#26 .

@leafpetersen leafpetersen assigned leafpetersen and unassigned lrhn Jul 25, 2018
@leafpetersen
Copy link
Member Author

Once we have a successful roll to flutter this can be closed.

@kevmoo
Copy link
Member

kevmoo commented Jul 25, 2018 via email

@leafpetersen
Copy link
Member Author

This is done, we've rolled flutter and internal.

@tonyclickspace
Copy link

chrome.dart still has instances of JSON.decode:
dart-gde/chrome.dart#278
Not sure if that's high enough priority to re-open this.

@leafpetersen
Copy link
Member Author

There's an open PR for chrome.dart (dart-gde/chrome.dart#272), but the scope of this bug is packages used directly by flutter or internal code which I believe is now complete.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-core-library SDK core library issues (core, async, ...); use area-vm or area-web for platform specific libraries.
Projects
None yet
Development

No branches or pull requests