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

Remove podspec lint bash script, use flutter_plugin_tools #2346

Merged
merged 5 commits into from
Apr 18, 2020

Conversation

jmagman
Copy link
Member

@jmagman jmagman commented Dec 4, 2019

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
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
@@ -124,3 +122,4 @@ task:
- 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

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

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

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
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 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
EdwinRomelta pushed a commit to EdwinRomelta/plugins that referenced this pull request Jun 11, 2020
jorgefspereira pushed a commit to jorgefspereira/plugins_flutter that referenced this pull request Oct 10, 2020
FlutterSu pushed a commit to FlutterSu/flutter-plugins that referenced this pull request Nov 20, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Move pod lib lint pre-submit plugin tests to flutter_plugin_tools
3 participants