Skip to content

Conversation

JDDV
Copy link
Contributor

@JDDV JDDV commented Jan 20, 2022

✨ What kind of change does this PR introduce? (Bug fix, feature, docs update...)

  • Updated pubspec.yaml to newer versions of SDK and flutter and adding flutter_lints as a dependency;
  • Updated analysis_options.yaml for using flutter_lints;
  • Updated code according to the set lint rule(s);
  • Updated compileSdkVersion and targetSdkVersion in the build.gradle
  • Fixed bug were starting the example app would close on running for the first time by adding internet permission in the AndroidManifest

⤵️ What is the current behavior?

🆕 What is the new behavior (if this is a feature change)?

💥 Does this PR introduce a breaking change?

No

🐛 Recommendations for testing

📝 Links to relevant issues/docs

🤔 Checklist before submitting

  • All projects build
  • Follows style guide lines (code style guide)
  • Relevant documentation was updated
  • Rebased onto current develop

@JDDV JDDV requested a review from mvanbeusekom January 20, 2022 15:19
@mvanbeusekom
Copy link
Member

@JDDV, I think you can fix the analyser error by adding the flutter_lints: ^1.0.4 dependency to the pubspec.yaml file inside the example folder.

@JDDV
Copy link
Contributor Author

JDDV commented Jan 20, 2022

@JDDV, I think you can fix the analyser error by adding the flutter_lints: ^1.0.4 dependency to the pubspec.yaml file inside the example folder.

I did that first as a fix, but I changed it to adding these lines in the option_analysis.yaml:

# workaround for https://github.com/dart-lang/sdk/issues/42910
    - 'example/**'

since that how it initialy was, should I change it back?

@mvanbeusekom
Copy link
Member

mvanbeusekom commented Jan 20, 2022

I did that first as a fix, but I changed it to adding these lines in the option_analysis.yaml:

# workaround for https://github.com/dart-lang/sdk/issues/42910
    - 'example/**'

since that how it initialy was, should I change it back?

Yes I think you should only add the dependency to the example/pubspec.yaml file but do not add a separate analysis_options.yaml file (I noticed you added this file as well in your first attempt to fix the error).

And make sure to remove the - 'example/**' line from the root analysis_options.yaml file.

@JDDV
Copy link
Contributor Author

JDDV commented Jan 20, 2022

I did that first as a fix, but I changed it to adding these lines in the option_analysis.yaml:

# workaround for https://github.com/dart-lang/sdk/issues/42910
    - 'example/**'

since that how it initialy was, should I change it back?

Yes I think you should only add the dependency to the example/pubspec.yaml file but do not add a separate analysis_options.yaml file (I noticed you added this file as well in your first attempt to fix the error).

And make sure to remove the - 'example/**' line from the root analysis_options.yaml file.

Done

@codecov
Copy link

codecov bot commented Jan 28, 2022

Codecov Report

Merging #27 (4be9888) into master (acd40d1) will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff            @@
##            master       #27   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files            2         2           
  Lines           13        12    -1     
=========================================
- Hits            13        12    -1     
Impacted Files Coverage Δ
lib/src/google_api_availability.dart 100.00% <ø> (ø)
.../src/models/google_play_services_availability.dart 100.00% <ø> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update acd40d1...4be9888. Read the comment docs.

JDDV and others added 4 commits January 28, 2022 10:48
Co-authored-by: Maurits van Beusekom <maurits@vnbskm.nl>
Co-authored-by: Maurits van Beusekom <maurits@vnbskm.nl>
Co-authored-by: Maurits van Beusekom <maurits@vnbskm.nl>
Co-authored-by: Maurits van Beusekom <maurits@vnbskm.nl>
Copy link
Member

@mvanbeusekom mvanbeusekom left a comment

Choose a reason for hiding this comment

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

LGTM

@mvanbeusekom mvanbeusekom merged commit c186fe7 into master Jan 28, 2022
@mvanbeusekom mvanbeusekom deleted the update_google_api_availability_plugin branch January 28, 2022 11:00
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.

2 participants