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.

Sign up for GitHub

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

Jump to bottom

Adding webdev in DEPS #26395

Merged
merged 2 commits into from
May 25, 2021
Merged

Adding webdev in DEPS #26395

merged 2 commits into from
May 25, 2021

Conversation

jonasfj
Copy link
Member

@jonasfj jonasfj commented May 25, 2021

Bringing in new deps from sdk/+/200920.

Cargo-culting here... I'm not entirely sure how to do this, but it seems DEPS didn't get automatically rolled. Because using webdev in recent pub fixes broke golem, so I had to rollback: sdk/+/201000.

I'd really like to land this, so I can roll pub forward.

@google-cla google-cla bot added the cla: yes label May 25, 2021
@jonasfj jonasfj requested a review from dnfield May 25, 2021 13:51
@jonasfj
Copy link
Member Author

jonasfj commented May 25, 2021

Filed sdk/+/201260 which I would like to land as soon as this is merged.

I'm not sure what's wrong with "Linux Web Framework tests" the details says: "Build infra failed".

@dnfield
Copy link
Contributor

dnfield commented May 25, 2021

You seem to be touching a part that the dart roller expects to manage. Do we need to update the autoroller for this? @bkonyi

/Cc @zanderso

@zanderso
Copy link
Member

Can you clarify what happened? Did the autoroller remove the entry for webdev from the Flutter DEPS file because it thought it was unused, and now you are manually adding it back? Or did the entry in the Flutter DEPS file never exist and now you are adding it for the first time?

If the latter, then this change lgtm. If you are feeling paranoid, you can manually verify that the entry has been added correctly with the script: https://github.com/flutter/buildroot/blob/master/tools/dart/create_updated_flutter_deps.py, which is what the autoroller uses to do the updates.

@jonasfj
Copy link
Member Author

jonasfj commented May 25, 2021 •
edited
Loading

Or did the entry in the Flutter DEPS file never exist and now you are adding it for the first time?

webdev have not been used in the Dart SDK before -- as far as I know :D
I would like to use it, so we introduced it in sdk/+/200920, specifically to allow the frontend_server_client package from the webdev repository to be used in pub.

So I think this is the latter case... I'm guessing that auto-roller doesn't add/remove DEPS from Dart SDK... merely update the existing revisions through simple search/replace..

@jonasfj
Copy link
Member Author

jonasfj commented May 25, 2021

Did a run of python2 ../tools/dart/create_updated_flutter_deps.py it only changed whitespace, so this is hopefully good to go.

@dnfield dnfield added the waiting for tree to go green This PR is approved and tested, but waiting for the tree to be green to land. label May 25, 2021
@fluttergithubbot fluttergithubbot merged commit bbe2925 into flutter:master May 25, 2021
@jonasfj jonasfj deleted the webdev-in-deps branch May 25, 2021 17:37
dart-bot pushed a commit to dart-lang/sdk that referenced this pull request May 25, 2021
@jonasfj
Reland "Bump pub."
1ee9b55 
This is a reland of 5c9e37f
Following update of `DEPS` in `flutter/engine`:
  flutter/engine#26395

New commits include:
```
git log --format="%C(auto) %h %s" 00c00e8adf9706bebe8f94483b7663c5f36f59d2..def32ceb1d660552eaec24839d377199aea5a569
 def32ceb Revert "Revert "Use the frontend server to compile pub executables (#2968)" (#3006)" (#3008)
 0dc7e50c Make `.gitignore` and `.pubignore` case-insensitive on Windows / MacOS (#3009)
 e89d4ab8 Generate GNU-style long file names in tar archives (#3005)
 d2ad13d0 Revert "Use the frontend server to compile pub executables (#2968)" (#3006)
 492b15ba New binstubs for global activate using `dart pub` (#3002)
 e02f23bb fix grammer in .gitignore error message (#2995)
 e01e3a41 Use the frontend server to compile pub executables (#2968)
 647989c6 Use RetryClient from package:http/retry.dart (#2980)
```

Original change's description:
> Change-Id: I9a0b5fb2b5616a5a0eaa16f3a90205bb1bb1fc8f
> Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/200878
> Auto-Submit: Jonas Jensen <[email protected]>
> Commit-Queue: Alexander Thomas <[email protected]>
> Reviewed-by: Alexander Thomas <[email protected]>

Change-Id: I1a0570318c63ec97fd141d79b405f55105111077
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/201260
Commit-Queue: Jonas Jensen <[email protected]>
Reviewed-by: Alexander Thomas <[email protected]>
@engine-flutter-autoroll engine-flutter-autoroll mentioned this pull request May 26, 2021
Merged
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request May 26, 2021
@engine-flutter-autoroll
bbe2925 Adding webdev in DEPS (flutter/engine#26395)
1009a26
naudzghebre pushed a commit to naudzghebre/engine that referenced this pull request Sep 2, 2021
@jonasfj
Adding webdev in DEPS (flutter#26395)
5aa5298
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@zanderso zanderso zanderso approved these changes

@dnfield dnfield Awaiting requested review from dnfield

Assignees
No one assigned
Labels
cla: yes waiting for tree to go green This PR is approved and tested, but waiting for the tree to be green to land.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@jonasfj @dnfield @zanderso @fluttergithubbot