Skip to content

TBR: Add getSupportedProtocols to VM Service Wrapper + pin webdev#2062

Merged
DanTup merged 6 commits intoflutter:masterfrom
DanTup:add-get-supported-protocols
Jun 10, 2020
Merged

TBR: Add getSupportedProtocols to VM Service Wrapper + pin webdev#2062
DanTup merged 6 commits intoflutter:masterfrom
DanTup:add-get-supported-protocols

Conversation

@DanTup
Copy link
Copy Markdown
Contributor

@DanTup DanTup commented Jun 10, 2020

v4.1.0 of vm_service adds a new method (getSupportedProtocols). I don't think this is technically a breaking change, however both DevTools and webdev implement the VmServiceInterface which means they don't compile with that version.

This change:

  1. Pins the webdev version activated on Travis back to version 2.5.6 which compiles (it gets vm_service v3)
  2. Requires v4.1 of vm_service for DevTools so we can access the new types, and provides an implementation for getSupportedProtocols to address the same issue for DevTools

If this goes green, I may land TBR so I can rebase my other PR and ensure it's passing, but we'll want to revert the hard-coded webdev version here as soon as webdev publishes a fix.

Related:

@DanTup DanTup changed the title Add get supported protocols Add getSupportedProtocols to VM Service Wrapper + pin webdev Jun 10, 2020
@gmpassos
Copy link
Copy Markdown

@DanTup
Copy link
Copy Markdown
Contributor Author

DanTup commented Jun 10, 2020

Pushed an additional change to add IDs to several constructors that now require them.

@gmpassos

Add pub upgrade to ensure that caches are not hiding any issue with new dependencies

The pubspec.lock file isn't committed here, so I believe the pub get call will already fetch the latest (allowed) versions from pub. I think pub upgrade is only required to check for newer versions than are in the lock file.

You need to fix this too:

Yeah, those are in other repositories and will need fixing there. I'm not entirely sure what the implementation of getSupportedProtocols will be for webdev so that's probably best done by the webdev developers. Here I'm just trying to get this build back green (the changes here roll back to an older version of webdev (temporarily) to avoid the issue until it's fixed in webdev).

@DanTup DanTup changed the title Add getSupportedProtocols to VM Service Wrapper + pin webdev TBR: Add getSupportedProtocols to VM Service Wrapper + pin webdev Jun 10, 2020
@DanTup
Copy link
Copy Markdown
Contributor Author

DanTup commented Jun 10, 2020

@jacob314 landing this TBR to get the build green - I'll address any feedback in a follow-up PR. The webdev version in travis.sh can be reverted once webdev is fixed, though all the other changes are required (to implement the updated VM Service interface, and provide IDs to all constructors that now mark them as required).

@DanTup DanTup merged commit f719729 into flutter:master Jun 10, 2020
@DanTup DanTup deleted the add-get-supported-protocols branch June 10, 2020 10:00
@DanTup
Copy link
Copy Markdown
Contributor Author

DanTup commented Jun 15, 2020

@jacob314 @devoncarew I landed this TBR. The hard-coded webdev version has since been reverted, but the other changes remain (required to implement the new vm_service interface) so still need to be reviewed. Thanks!

Copy link
Copy Markdown
Member

@kenzieschmoll kenzieschmoll left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm_army

@DanTup
Copy link
Copy Markdown
Contributor Author

DanTup commented Jun 15, 2020

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants