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

Remove podspec lint bash script, use flutter_plugin_tools #2346

Merged
jmagman merged 5 commits into from
Apr 18, 2020
Merged

Remove podspec lint bash script, use flutter_plugin_tools #2346

jmagman merged 5 commits into from
Apr 18, 2020

Conversation

@jmagman
Copy link
Member

@jmagman jmagman commented Dec 4, 2019 •
edited
Loading

Description

Remove lint podspec bash script, use flutter_plugin_tools podspecs command.

Related Issues

Command introduced with https://github.com/flutter/plugin_tools/pull/69.
Fixes flutter/flutter#41511

Checklist

Before you create this PR confirm that it meets all requirements listed below by checking the relevant checkboxes ([x]). This will ensure a smooth and quick review process.

  • I read the [Contributor Guide] and followed the process outlined there for submitting PRs.
  • My PR includes unit or integration tests for all changed/updated/fixed behaviors (See [Contributor Guide]).
  • All existing and new tests are passing.
  • I updated/added relevant documentation (doc comments with ///).
  • The analyzer (flutter analyze) does not report any problems on my PR.
  • I read and followed the [Flutter Style Guide].
  • The title of the PR starts with the name of the plugin surrounded by square brackets, e.g. [shared_preferences]
  • I updated pubspec.yaml with an appropriate new version according to the [pub versioning philosophy].
  • I updated CHANGELOG.md to add a description of the change.
  • I signed the [CLA].
  • I am willing to follow-up on review comments in a timely manner.

Breaking Change

  • Yes, this is a breaking change (please indicate a breaking change in CHANGELOG.md and increment major revision).
  • No, this is not a breaking change.

@jmagman jmagman requested a review from mklim December 4, 2019 21:17
@jmagman jmagman requested a review from cyanglaz December 4, 2019 21:18
mklim
mklim approved these changes Dec 4, 2019
View reviewed changes
Copy link
Contributor

@mklim mklim left a comment

Choose a reason for hiding this comment

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

LGTM

.cirrus.yml Outdated
- flutter channel $CHANNEL
- ./script/incremental_build.sh build-examples --ipa
- ./script/incremental_build.sh drive-examples
- ./script/incremental_build.sh podspecs --skip url_launcher_web --skip google_sign_in_web --skip e2e_macos --skip video_player_web --no-analyze camera
Copy link
Contributor

@mklim mklim Dec 4, 2019

Choose a reason for hiding this comment

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

nit: Would it be worth doing this before any of the other steps? That way if there's a lint error this will fail immediately instead of waiting until everything else completes.

Copy link
Member Author

@jmagman jmagman Dec 4, 2019

Choose a reason for hiding this comment

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

Or maybe I should split it into its own shard so if one fails you still get the output from the others?

Copy link
Contributor

@mklim mklim Dec 4, 2019

Choose a reason for hiding this comment

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

That makes sense too. I think either is fine.

@jmagman jmagman force-pushed the plugin-lint branch 2 times, most recently from ae094c2 to 513afa2 Compare December 5, 2019 19:09
@jmagman
Copy link
Member Author

jmagman commented Dec 5, 2019

I fixed the new prefer_single_quotes warnings, but I don't want to fix the omit_local_variable_types since I think that will go away once packages/google_sign_in/google_sign_in//analysis_options.yaml gets removed (see https://github.com/flutter/plugin_tools/pull/75)
https://cirrus-ci.com/task/5563407964569600?command=main#L50

@jmagman
Copy link
Member Author

jmagman commented Dec 5, 2019

I fixed the new prefer_single_quotes warnings, but I don't want to fix the omit_local_variable_types since I think that will go away once packages/google_sign_in/google_sign_in//analysis_options.yaml gets removed (see flutter/plugin_tools#75)
https://cirrus-ci.com/task/5563407964569600?command=main#L50

Also I'm not sure know why they just now surfaced, those double quotes are from 2017.

@jmagman jmagman changed the title [google_sign_in] Remove podspec lint bash script, use flutter_plugin_tools, fix analyzer warning [google_sign_in] Remove podspec lint bash script, use flutter_plugin_tools, fix analyzer warnings Dec 5, 2019
@mklim
Copy link
Contributor

mklim commented Dec 5, 2019

@jmagman it's because I didn't lock the pedantic package and it silently upgraded to 1.9.0 underneath CI. I have an open PR right now to lock it back down to 1.8.0. #2354

@jmagman
Copy link
Member Author

jmagman commented Dec 5, 2019

@jmagman it's because I didn't lock the pedantic package and it silently upgraded to 1.9.0 underneath CI. I have an open PR right now to lock it back down to 1.8.0. #2354

🤦‍♀ I forgot it too in plugin_tools https://github.com/flutter/plugin_tools/pull/76

@jmagman jmagman force-pushed the plugin-lint branch 2 times, most recently from dd7af5c to aa7e621 Compare December 7, 2019 02:57
@jmagman
Copy link
Member Author

jmagman commented Dec 9, 2019

This has been good and truly borked by federated plugins, trying to wack-a-mole errors as they get merged.

@jmagman jmagman force-pushed the plugin-lint branch 2 times, most recently from 4b43899 to 6e5e731 Compare December 10, 2019 20:29
@cyanglaz cyanglaz mentioned this pull request Jan 7, 2020
Merged
13 tasks
@jmagman
Copy link
Member Author

jmagman commented Jan 7, 2020

I can see the error now with https://github.com/flutter/plugin_tools/pull/84!

 -> google_sign_in (0.0.1)
    - WARN  | license: Missing license type.
    - WARN  | [iOS] keys: Missing primary key for `source` attribute. The acceptable ones are: `git, hg, http, svn`.
    - ERROR | [iOS] unknown: Encountered an unknown error (/bin/bash -c 
set -e
unzip Flutter.framework -d Flutter.framework
Archive:  Flutter.framework.zip
Archive:  Flutter.framework.ZIP
  End-of-central-directory signature not found.  Either this file is not
  a zipfile, or it constitutes one disk of a multi-part archive.  In the
  latter case the central directory and zipfile comment will be found on
  the last disk(s) of this archive.
  End-of-central-directory signature not found.  Either this file is not
  a zipfile, or it constitutes one disk of a multi-part archive.  In the
  latter case the central directory and zipfile comment will be found on
  the last disk(s) of this archive.
) during validation.

The Flutter.framework.zip is corrupted?

@jmagman jmagman mentioned this pull request Mar 10, 2020
Closed
jmagman referenced this pull request in firebase/flutterfire Mar 10, 2020
@collinjackson
Make plugin linter verbose to assist in troubleshooting failures
1bc6ea6
@jmagman jmagman changed the title [google_sign_in] Remove podspec lint bash script, use flutter_plugin_tools, fix analyzer warnings Remove podspec lint bash script, use flutter_plugin_tools Apr 17, 2020
@jmagman jmagman requested a review from blasten April 18, 2020 00:08
@jmagman jmagman merged commit ffc6824 into flutter:master Apr 18, 2020
@jmagman jmagman deleted the plugin-lint branch April 18, 2020 01:01
@jmagman jmagman mentioned this pull request Apr 20, 2020
Closed
EdwinRomelta pushed a commit to EdwinRomelta/plugins that referenced this pull request Jun 11, 2020
@jmagman @EdwinRomelta
Remove podspec lint bash script, use flutter_plugin_tools (flutter#2346)
bd695e4
jorgefspereira pushed a commit to jorgefspereira/plugins_flutter that referenced this pull request Oct 10, 2020
@jmagman @jorgefspereira
Remove podspec lint bash script, use flutter_plugin_tools (flutter#2346)
bbfb19e
FlutterSu pushed a commit to FlutterSu/flutter-plugins that referenced this pull request Nov 20, 2020
@jmagman
Remove podspec lint bash script, use flutter_plugin_tools (flutter#2346)
2f96aff
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@cyanglaz cyanglaz Awaiting requested review from cyanglaz

@blasten blasten Awaiting requested review from blasten

1 more reviewer

@mklim mklim mklim approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

cla: yes

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Move pod lib lint pre-submit plugin tests to flutter_plugin_tools

3 participants

@jmagman @mklim @googlebot