Skip to content
This repository was archived by the owner on Apr 21, 2025. It is now read-only.

Conversation

@brunobowden
Copy link
Contributor

@brunobowden brunobowden commented Oct 25, 2020

Screenshots

Screenshots Add any relevant before/after screenshots here

How did you test the change?

Will test later with device turning on and off wifi access

  • iOS Simulator
  • iOS Device
  • Android Simulator
  • Android Device
  • curl to a dev App Engine server
  • other, please describe

Checklist:

@brunobowden
Copy link
Contributor Author

FYI @Mohit020888 - see if you can figure out a good way to test this

theswerd
theswerd previously approved these changes Oct 25, 2020
Copy link
Contributor

@theswerd theswerd left a comment

Choose a reason for hiding this comment

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

LGTM

@theswerd theswerd self-requested a review October 25, 2020 23:23
@theswerd theswerd dismissed their stale review October 25, 2020 23:23

gotta re-review because of CI Errors

@brunobowden brunobowden force-pushed the connectivity-no-location branch 4 times, most recently from 2cc6feb to 1bbc5b5 Compare October 26, 2020 05:50
@brunobowden
Copy link
Contributor Author

brunobowden commented Oct 26, 2020

@cyanglaz - thanks for the updating Connectivity to v2.0.0. You put a lot of care in to the deprecation message, v2.0.0 as a breaking change and so on. Sorry to trouble you further but I'd appreciate if you could take a look at this CI issue I'm seeing after the upgrade:

The conflict was with PR flutter/plugins#3042 where compileSdkVersion 29 was added in connectivity v0.4.9+5. I think the package may be fine but it's causing some interaction with our CI. Our CI uses Java 12 - see workflow. After the update, it fails on a Java warning for source value 7 is obsolete. That same warning occurred with connectivity v0.4.9+3 but it didn't fail the CI build.

I didn't want to take up too much of your time but I'd appreciate your perspective if you had any insights.

CI Build Error

Running Gradle task 'assembleRelease'...                        
warning: [options] source value 7 is obsolete and will be removed in a future release
warning: [options] target value 7 is obsolete and will be removed in a future release
warning: [options] To suppress warnings about obsolete options, use -Xlint:-options.
error: warnings found and -Werror specified
FAILURE: Build failed with an exception.
* What went wrong:
Execution failed for task ':connectivity:compileReleaseJavaWithJavac'.

@cyanglaz
Copy link

I'm not too familiar with Android stuff. But it looks like you could switch to android 29 on ci?

@brunobowden brunobowden force-pushed the connectivity-no-location branch from acbf22a to ec1f42f Compare October 27, 2020 23:45
@brunobowden
Copy link
Contributor Author

@cyanglaz - I changed compileSdkVersion from 28 to 29 as part of the PR. Still looking in to this.

brunobowden added a commit to brunobowden/app that referenced this pull request Oct 28, 2020
@brunobowden brunobowden mentioned this pull request Oct 28, 2020
7 tasks
Copy link
Contributor

@theswerd theswerd left a comment

Choose a reason for hiding this comment

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

LGTM

@brunobowden
Copy link
Contributor Author

@yukuairoy - needs your approval too

@brunobowden
Copy link
Contributor Author

brunobowden commented Oct 28, 2020

@cyanglaz - figured out the problem was that the connectivity plugin recently started treating all warnings as errors.... so a warning about Java 7 target becoming obsolete now causes the build to fail. Temporary workaround is to downgrade to Java 11 environment (which doesn't treat Java 7 target as obsolete). Proper fix is Java 8 target for all Flutter plugins that have the Werror flag: flutter/plugins#3216

- Closes WorldHealthOrganization#1532
- Replace 3rd party simple_connectivity with Google connectivity package
- New version removes dependency on locations permission:
- compileSdkVersion 29 - matching targetSdk and Flutter packages
- Java 11 for CI environment to fix build failure where `Werror` flag
in connectivity causes `source value 7 is obsolete` warning to fail build.
@brunobowden brunobowden force-pushed the connectivity-no-location branch from 77743f6 to 3f548bc Compare October 28, 2020 07:03
@brunobowden brunobowden merged commit 192fccf into WorldHealthOrganization:master Oct 28, 2020
brunobowden added a commit to brunobowden/app that referenced this pull request Oct 28, 2020
@brunobowden brunobowden deleted the connectivity-no-location branch December 25, 2020 00:51
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Connectivity no location permissions proper fix

4 participants