azavea / cac-tripplanner-android

GoPhillyGo Android app
https://gophillygo.org
Apache License 2.0
3 stars 3 forks source link

Update build.gradle and README for automatic signing configuration #212

Closed ktohalloran closed 1 year ago

ktohalloran commented 1 year ago

Overview

As a part of #206, I noticed some odd behavior with release.properties. After a bit of investigation, it turns out that release.properties was a configuration file for automatically signing apps while building, rather than following the signing instructions written up in the README. It was being ignored during the build process due to some outdated syntax. This PR restores this automatic signing configuration and updates the README to reflect the automatic signing process and explain the use of the release.properties file. It also updates the README to consolidate the testing instructions and adds closed testing as an option for the Google Play testing phase to reflect its use in #206.

Notes

The initial automatic signing configuration checked for the existence of release.properties before setting the signing config values or logged a warning if it didn't find it. I tried to replicate the check with this new syntax, but didn't try too hard when it failed. I think it's better for the build to fail loudly and locally throw an error than for it to quietly log a warning and then Google Play Console throw an error about missing signing values.

Testing Instructions

ktohalloran commented 1 year ago

@ddohler I think CircleCI's failure was a fluke, because it failed claiming that there was no compileSdkVersion specified, which it definitely is. But to make sure there's not a larger problem here, I'd like to re-run it but don't have permission. Are you able to re-run it?

ktohalloran commented 1 year ago

Well that's really strange. Ok, I'll look into the CircleCI config; if I can't figure out where/why it's trying to use that file, I'll look into restoring the way it used to work of failing quietly if the file didn't exist.

ktohalloran commented 1 year ago

I just can't seem to get this setup to work without release.properties. It's frustrating to me that CircleCI feels the need to sign our app when we don't even use it to release the app, but because of this, I don't think I can get automatic signing working without spending time trying to set up environment variables for CircleCI to use. This doesn't feel like a priority, though, so I can make a follow up issue for visibility if we think this will ever be worth the time, but for now, @ddohler I'm inclined to just close this PR without merging and keep using the original way we had singing apps. What do you think?

ddohler commented 1 year ago

@ktohalloran That makes sense to me, unfortunately -- the one thought I had was that perhaps it just wants the file to exist but won't actually use it, in which case maybe creating an empty file called release.properties in config.yml would be sufficient. But if you've already tried that then I'm all out of ideas and I agree we should close this one and stick with the original setup.

ktohalloran commented 1 year ago

Yeah it doesn't like an empty file, either. Alright, well, that's annoying. Closing now.