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

Option to require Wi-Fi for auto-upload #106

Merged
merged 9 commits into from
Mar 1, 2021
Merged

Option to require Wi-Fi for auto-upload #106

merged 9 commits into from
Mar 1, 2021

Conversation

Mortein
Copy link
Contributor

@Mortein Mortein commented Feb 27, 2021

Added option in Settings to require Wi-Fi for auto-uploader (#101).

When auto-upload background routine runs, it uses the Connectivity plugin to check for Wi-Fi, if it's required.

@thielepaul
Copy link
Owner

Looks good to me. However, the CI build is failing. Do you know why? If its working for you can you check if the CI uses the same flutter version (https://github.com/photoprism/photoprism-mobile/blob/master/.github/workflows/pull-request.yml)?

@Mortein
Copy link
Contributor Author

Mortein commented Feb 27, 2021

I'm using the VSCode Docker container, so 1.22.4 vs. 1.22.6 which CI seems to be using.

Doing a flutter clean, then running the same build command flutter build apk --profile --flavor dev --no-shrink throws the same deprecation warnings as CI does (in both versions). Without a clean, it actually builds without warnings.

For flutter_secure_storage, I've found mogol/flutter_secure_storage#177 but it hasn't yet been released, so 3.3.5 (which is in use) is as high as we can go.

For file_picker, there are newer packages (1.4.3 vs. 2.1.6), but it'll need a bit of rework to the code. Version 2+ of file_picker also supports FileType.media to show the media picker instead, which would solve #73.

@thielepaul
Copy link
Owner

thanks for looking into it, I updated the filepicker version in #96.
Would it be possible to disable -Werror here?

@thielepaul
Copy link
Owner

have you tried this? https://stackoverflow.com/a/64957466

@Mortein
Copy link
Contributor Author

Mortein commented Feb 28, 2021

Good find, but unfortunately it just tells us a bit more about the deprecated objects, and which lines are using them (flutter_secure_storage and file_picker).

I think the issue may have been happening for a while (it's in some older build pipelines, pre-January) by a bump to flutter dependencies that caused the minimum SDK version to change to 29, which is where the deprecations started. I think the Android compile SDK should be changed to 29 (to match the dependencies), and the Docker flutter image to :stable (currently 1.22.6) so we don't get out of sync with the CI (1.22.x, which is 1.22.6).

Your link did lead me on to Gradle's lint baseline to exclude specific warnings and ignore them until they're fixed, but it'll have to wait until after work before I can try it out.

Hopefully works as expected...
Dev container uses 8
Names for steps
Always run build job
Names for package steps
Don't analyze/format package (as build does it)
Remove coverage for build (as not used)
CI for non-PR, and partially for other repos
@Mortein Mortein marked this pull request as draft March 1, 2021 18:43
Should otherwise be tested by the pull request build
Revert to earlier CI, but fix issue
@thielepaul
Copy link
Owner

looks good to me, if you mark the PR as ready, I'll merge it

@Mortein
Copy link
Contributor Author

Mortein commented Mar 1, 2021

Modified build job in push workflow to only run on master, so I was able to test the build job in pull request workflow in another repo/branch. Lowered the JDK version from 12 to 8 in both CI workflows to match the dev container, to fix the issue that was occurring.

Deprecation warnings are still happening, but once flutter_secure_storage has a new release, and file_picker is updated as part of #96, they'll go away.

@Mortein Mortein marked this pull request as ready for review March 1, 2021 19:23
@thielepaul thielepaul merged commit 6c3ce15 into thielepaul:master Mar 1, 2021
@Mortein Mortein deleted the auto-upload-wifi branch March 2, 2021 07:41
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.

None yet

2 participants