Skip to content
This repository has been archived by the owner on Feb 22, 2023. It is now read-only.

[google_sign_in] remove execute bit from a lot of files #3848

Merged
merged 2 commits into from
May 7, 2021
Merged

[google_sign_in] remove execute bit from a lot of files #3848

merged 2 commits into from
May 7, 2021

Conversation

kevmoo
Copy link
Contributor

@kevmoo kevmoo commented May 4, 2021 •
edited
Loading

No description provided.

@google-cla google-cla bot added the cla: yes label May 4, 2021
@kevmoo kevmoo requested review from ditman and stuartmorgan May 4, 2021 20:10
stuartmorgan
stuartmorgan reviewed May 4, 2021
View reviewed changes
packages/google_sign_in/google_sign_in/integration_test/google_sign_in_test.dart Outdated
@@ -2,8 +2,6 @@
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.

// @dart = 2.9
Copy link
Contributor

Choose a reason for hiding this comment

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

How is this passing tests? integration_test isn't null safe on stable.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

🤷 – is CI running on stable?

Copy link
Member

Choose a reason for hiding this comment

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

I think this is passing tests, because the integration tests are not running for this package.

The location of the test file is not standard (it should live inside: google_sign_in/google_sign_in/example/...)

Copy link
Contributor

Choose a reason for hiding this comment

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

flutter/flutter#81929 😢

ditman
ditman reviewed May 5, 2021
View reviewed changes
Copy link
Member

@ditman ditman left a comment

Choose a reason for hiding this comment

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

I'd revert the @Dart=2.9 for now, until the tests are fully migrated, running and supported in stable.

packages/google_sign_in/google_sign_in/integration_test/google_sign_in_test.dart Outdated
@@ -2,8 +2,6 @@
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.

// @dart = 2.9
Copy link
Member

Choose a reason for hiding this comment

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

I think this is passing tests, because the integration tests are not running for this package.

The location of the test file is not standard (it should live inside: google_sign_in/google_sign_in/example/...)

@kevmoo
Copy link
Contributor Author

kevmoo commented May 5, 2021

I'd revert the @Dart=2.9 for now, until the tests are fully migrated, running and supported in stable.

ack

@kevmoo
[google_sign_in_web] misc cleanup
cb0fc49 
* Add missing integration_test dev dependency
* Remove execute bit from many files
@kevmoo
Copy link
Contributor Author

kevmoo commented May 5, 2021

PTAL!

@kevmoo
Copy link
Contributor Author

kevmoo commented May 5, 2021

Are the windows bots just flaky?

@ditman
Copy link
Member

ditman commented May 5, 2021

Are the windows bots just flaky?

@kevmoo I've seen them fail in flutter/engine today too, not entirely sure how bad it is.

@kevmoo
Copy link
Contributor Author

kevmoo commented May 6, 2021

I think this is generally good, but happy to drop it. I need to get some other content going RE documentation here.

@kevmoo kevmoo changed the title [google_sign_in_web] misc cleanup [google_sign_in] remove execute bit from a lot of files May 6, 2021
@ditman
Copy link
Member

ditman commented May 6, 2021

I think this is generally good, but happy to drop it. I need to get some other content going RE documentation here.

@kevmoo we're just waiting for the testing infra to recover to merge.

@kevmoo
Copy link
Contributor Author

kevmoo commented May 6, 2021

I think this is generally good, but happy to drop it. I need to get some other content going RE documentation here.

@kevmoo we're just waiting for the testing infra to recover to merge.

should I rebase?

@ditman
Copy link
Member

ditman commented May 6, 2021

@kevmoo no need, I think these builds are getting retried without much manual intervention (?)

https://ci.chromium.org/ui/p/flutter/builders/try/Windows%20Plugins/4026/related-builds

@stuartmorgan
Copy link
Contributor

I believe Windows desktop builds specifically are still broken, due to the way they changed images to solve the wider issue.

stuartmorgan
stuartmorgan approved these changes May 7, 2021
View reviewed changes
Copy link
Contributor

@stuartmorgan stuartmorgan left a comment

Choose a reason for hiding this comment

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

LGTM. Thanks for the cleanup; I wonder how they ended up this way.

@kevmoo kevmoo merged commit 26456c8 into master May 7, 2021
@kevmoo kevmoo deleted the google_sign_in_cleanup branch May 7, 2021 15:18
@engine-flutter-autoroll engine-flutter-autoroll mentioned this pull request May 7, 2021
Merged
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request May 7, 2021
@engine-flutter-autoroll
26456c8 [google_sign_in] remove execute bit from a lot of files (flut…
d643955 
…ter/plugins#3848)
fotiDim pushed a commit to fotiDim/plugins that referenced this pull request Sep 13, 2021
@kevmoo @fotiDim
[google_sign_in] remove execute bit from a lot of files (flutter#3848)
62b6a8b
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@ditman ditman ditman left review comments

@stuartmorgan stuartmorgan stuartmorgan approved these changes

Assignees
No one assigned
Labels
cla: yes p: google_sign_in platform-android platform-ios
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@kevmoo @ditman @stuartmorgan