Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fastlane update #576

Merged
merged 3 commits into from
Nov 8, 2022
Merged

Fastlane update #576

merged 3 commits into from
Nov 8, 2022

Conversation

MagicLike
Copy link
Contributor

Updated screenshots, icon and feature graphic (related to #521)

Updated screenshots, icon and feature graphic
@MagicLike
Copy link
Contributor Author

@IzzySoft is it possible, that the description in the repo here doesn't match with the description in your repo?

@IzzySoft
Copy link

IzzySoft commented Nov 6, 2022

Everything is possible. Let me check… Yes, my updater ignores descriptions due to formatting issues (I've adjusted that at my end and then locked it). Checking again now: Still the same. I thought I had mentioned it once. Compare what's shown in my repo against how the current full_description.txt would render:


Authenticator Pro generates Two Factor Authentication (2FA) codes for your online accounts. TOTP, HOTP, mOTP (Mobile-OTP) and Steam are supported. The generated codes are one time tokens that provide an extra layer of security to your online accounts. After scanning a simple QR code, your account is protected.

Please direct all feedback / issues to either GitHub or email (developer contact section) instead of writing a review to more easily follow up on your issue.

Frequently Asked Questions: https://github.com/jamie-mh/AuthenticatorPro/wiki#frequently-asked-questions

Compatibility Authenticator Pro is compatible with most providers and accounts.

Backup / Restore Backup your authenticators with strong encryption. In case you lose your or change phone, you can always gain access to your accounts. Save to cloud storage or to your device.

Import Easily migrate your accounts to Authenticator Pro from supported alternative apps.


For some formatting hints, please see here. If you prefer the description should be used as shown above, I can of course switch – but maybe you want to give it a little polish first?

@MagicLike
Copy link
Contributor Author

For some formatting hints, please see here. If you prefer the description should be used as shown above, I can of course switch – but maybe you want to give it a little polish first?

Yes, that is what I thought of doing... Are Emojis supported?

@MagicLike
Copy link
Contributor Author

@IzzySoft Ok, I updated the descriptions...

But one thing I spotted on the app-page in F-Droid: the licence is MIT, although this project is now under the GPL-3.0 licence. Is there a way to change that?

@MagicLike
Copy link
Contributor Author

@jamie-mh I also changed the formatting of the fastlane descriptions to markdown for now.

@IzzySoft
Copy link

IzzySoft commented Nov 6, 2022

Are Emojis supported?

Yes, as long as you're not overdoing it 🤣 e.g. don't use them as "fancy bullet points" if it would break the markdown bullet-point list.

But one thing I spotted on the app-page in F-Droid: the licence is MIT, although this project is now under the GPL-3.0 licence. Is there a way to change that?

Yes. Verified and done. If such data changes (which is not part of Fastlane), just drop me a note, that needs to be done manually.

I also changed the formatting of the fastlane descriptions to markdown for now.

Eh, how did I guess your intention of emojis before looking at that? 🤣 Doesn't break the formatting, so it's OK. Thanks!

Question: The full description in Fastlane says it's GPL-3.0-or-later. No such hint on the Readme. Not really a "deal breaker", but might be a bit confusing (just updated metadata here again to reflect that). Might be a good idea to add that disclaimer at the end of the Readme as well.

Updated the config here to pull full-description along. Didn't trigger that yet as this PR is not yet merged. So 🤞 the merge will happen before the next release 😉

@MagicLike
Copy link
Contributor Author

Question: The full description in Fastlane says it's GPL-3.0-or-later. No such hint on the Readme. Not really a "deal breaker", but might be a bit confusing (just updated metadata here again to reflect that). Might be a good idea to add that disclaimer at the end of the Readme as well.

@IzzySoft Done! Will now convert the PR...

@MagicLike MagicLike marked this pull request as ready for review November 6, 2022 15:26
@jamie-mh
Copy link
Member

jamie-mh commented Nov 8, 2022

Looks good. Thanks for working on this!
These assets haven't been updated in a while.

@jamie-mh jamie-mh merged commit d55518c into stratumauth:master Nov 8, 2022
@IzzySoft
Copy link

IzzySoft commented Nov 8, 2022

Will be updated automatically in my repo then with the next release. If that's too far into the future for you let me know and I trigger it manually.

@MagicLike
Copy link
Contributor Author

@jamie-mh Is it possible in Crowdin to mark certain characters that shouldn't be translated?
(I am in the process of translating the Fastlane details completely into German, but it bothers me that the Markdown formatting characters are being included in the translations).

@MagicLike
Copy link
Contributor Author

MagicLike commented Nov 19, 2022

@jamie-mh Please merge #591 soon, so that it is corrected until the next release & I can complete the German translation fully.

@MagicLike
Copy link
Contributor Author

@jamie-mh I've adjusted the translations a bit (again 😄).... It's now written in the 2nd form singular instead of the 3rd form plural, which makes it a bit more personal. Please approve these over on Crowdin.
grafik

@MagicLike
Copy link
Contributor Author

@IzzySoft Just a short question: How long does it take for an app in your repo to be updated? (The new release is out by two days now and there is no update over there...)

@IzzySoft
Copy link

IzzySoft commented Feb 4, 2023

Should be picked up within 24h (unless auto-updates are explicitly disabled or set to static, where static means to check monthly instead of daily). 2+ days and no update means something's wrong, let me check… Ooof. APK size doubled and you're now at 2 times the per-app limit. Didn't know my updater would reject it then (which I think is just an accident – caused by you no longer providing the matching APKs: it's set to pick only /armeabi/i), but now your app is no longer meeting the inclusion criteria. Could you provide per-ABI builds and let me know whether to pick up the armeabi-v7a or the arm64-v8 variant then again? That would bring us back into limits. Oh, and then there's be another constraint:

Offending libs:
---------------
* Firebase Data Transport (/com/google/android/datatransport): NonFreeNet
* Android Wear APIs (/com/google/android/gms/wearable): NonFreeDep
* Google Mobile Services (/com/google/android/gms): NonFreeDep
* Firebase (/com/google/firebase): NonFreeNet,NonFreeDep
* ML Kit (/com/google/mlkit): NonFreeDep,Tracking

5 offenders.

You now include ML Kit, which is proprietary and drags in even more proprietary components, so we have another show-stopper. The previous versions came with GMS and Wear already. I accepted that as it's in the "gray zone" (I can close an eye or two, but I do not have 5 eyes). So we'd need a build flavor coming without those.

So sad to note, but to make sure I've disabled auto-updates right now (wasn't disabled until a few minutes ago; I always inform the authors in such cases and give reasons as well as possible solutions) until these issues are solved. Hope you can solve both 🤞

@MagicLike
Copy link
Contributor Author

Tagging @jamie-mh
Please have a look at this 👆

@MagicLike
Copy link
Contributor Author

Should be picked up within 24h (unless auto-updates are explicitly disabled or set to static, where static means to check monthly instead of daily). 2+ days and no update means something's wrong, let me check… Ooof. APK size doubled and you're now at 2 times the per-app limit. Didn't know my updater would reject it then (which I think is just an accident – caused by you no longer providing the matching APKs: it's set to pick only /armeabi/i), but now your app is no longer meeting the inclusion criteria. Could you provide per-ABI builds and let me know whether to pick up the armeabi-v7a or the arm64-v8 variant then again? That would bring us back into limits. Oh, and then there's be another constraint:

Offending libs:
---------------
* Firebase Data Transport (/com/google/android/datatransport): NonFreeNet
* Android Wear APIs (/com/google/android/gms/wearable): NonFreeDep
* Google Mobile Services (/com/google/android/gms): NonFreeDep
* Firebase (/com/google/firebase): NonFreeNet,NonFreeDep
* ML Kit (/com/google/mlkit): NonFreeDep,Tracking

5 offenders.

You now include ML Kit, which is proprietary and drags in even more proprietary components, so we have another show-stopper. The previous versions came with GMS and Wear already. I accepted that as it's in the "gray zone" (I can close an eye or two, but I do not have 5 eyes). So we'd need a build flavor coming without those.

So sad to note, but to make sure I've disabled auto-updates right now (wasn't disabled until a few minutes ago; I always inform the authors in such cases and give reasons as well as possible solutions) until these issues are solved. Hope you can solve both crossed_fingers

Oops, I did not know about that...

@IzzySoft
Copy link

IzzySoft commented Feb 4, 2023

Oops, I did not know about that...

Which part? 🙈

@MagicLike
Copy link
Contributor Author

MagicLike commented Feb 4, 2023

Oops, I did not know about that...

Which part? see_no_evil

About the new packages, the apk size, etc,,,, - everything that you wrote about ^^

@IzzySoft
Copy link

IzzySoft commented Feb 4, 2023

Oh. But that's quite visible, isn't it? 🤪 (overlooked I guess) And you had per-ABI builds before. I'd need to check whether and where we talked about which of them I'd take – but I had to fix it to one ABI, else the updates wouldn't match (would be bad if you'd installed an ARM version, and suddenly there are only x86 updates, no?) 😉

@jamie-mh
Copy link
Member

jamie-mh commented Feb 4, 2023

Hi there,

It's not a problem to remove all proprietary components with a build flag, including the Wear OS/GMS stuff.
However, since the migration from Xamarin to .NET Android, even the individual APKs are around 35 Mb in size (without aforementioned libs), since everything is AOT compiled. Is this a deal-breaker, or can the limit be raised slightly?

Cheers

@MagicLike
Copy link
Contributor Author

Oh. But that's quite visible, isn't it? zany_face (overlooked I guess) And you had per-ABI builds before. I'd need to check whether and where we talked about which of them I'd take – but I had to fix it to one ABI, else the updates wouldn't match (would be bad if you'd installed an ARM version, and suddenly there are only x86 updates, no?) wink

Actually I am not a dev on this project, I do the German translations & the README/Fastlane so I did not even look out for it... 😄

@IzzySoft
Copy link

IzzySoft commented Feb 4, 2023

@jamie-mh

It's not a problem to remove all proprietary components with a build flag, including the Wear OS/GMS stuff.

That sounds good!

However, since the migration from Xamarin to .NET Android, even the individual APKs are around 35 Mb in size (without aforementioned libs)

Oof, that's a bummer. Better than the almost 60 MB, but still… Would minification help? Like, Minify android app but do not obfuscate it? Or is that not possible with DotNet? I'm not… err, see my response to Magic below 🙈 … hrn, so I cannot really tell. I just collect useful ideas here and there…

Is this a deal-breaker, or can the limit be raised slightly?

There's actually a "hard limit" in one of the APIs I use which gets hit at 32 MB. I found a work-around for that, but would like to keep the ball low – so I cannot raise the limit generally (which would also contradict the goal of keeping APK size "in bounds", I always have folks with low-end devices and low data plans in mind), but I can make exceptions here and there.

So let's strike a deal: You create that flavor/build-flag keeping the proprietary libs out and attach the resulting per-ABI builds (for me, the ARM ones suffice – but no harm in having the others there, too) and try to keep them as compact as possible (ideally without much future growth in size – but we both know that it will grow which can't be helped) – and I'll close an eye on the 30M limit for your app, allowing for another 10..15 MB in the hope it won't grow beyond 45 MB (let's call that the "50% bonus award" 🤪). Deal?

If you agree, please let me know what file names you will take. You could e.g. let the "fat lady" sit with all the stuff in as it is on the current last release (I'd ignore that anyway) and add the per-ABI builds only for the "lite build", so we're back at the original file name pattern. My updater then would continue looking out for the /armeabi/i match and pick that.

@MagicLike

Actually I am not a dev on this project

Hey, klau doch nicht einfach meinen Standard-Disclaimer 🤣 ("I'm not an Android dev…" is how many of my responses start, see above 🙈)

@jamie-mh
Copy link
Member

jamie-mh commented Feb 5, 2023

@IzzySoft

I've updated the latest release with slightly tweaked APKs built from the same source. The ones with a "fdroid" suffix, should match the F-Droid inclusion policy in terms of non-free dependencies: Wear OS sync removed, ML Kit barcode scanning replaced with ZXing.

Can you run a scan and let me know if you find anything?

Thanks

@IzzySoft
Copy link

IzzySoft commented Feb 5, 2023

Sure I can! And here are the results (from the armeabi build – but I expect no differences for the others):

No offending libs found.

Congrats, all "clean"! So which arch would you like me to take? I'd suggest I pick the armeabi fdroid variant (for widest compatibility), but I'd be fine taking the arm64 if you prefer (funnily, both have the same size – usually arm64 is slightly bigger). I'd then set it up and re-enable updates again.

PS: Do we need to adjust the description then, as not all features are available with the free/libre build?

@jamie-mh
Copy link
Member

jamie-mh commented Feb 5, 2023

Great! Let's stick to the armeabi variant again, you're right, it's the most compatible.
I've updated the description + FAQ with a little disclaimer on Wear OS compatibility.

image

@IzzySoft
Copy link

IzzySoft commented Feb 5, 2023

Agreed, and thanks – especially for the graphic! With your permission, may I show that to those doubting that my favoring of armeabi over arm64 really helps to a more wide compatibility? And if so, mind to name its source (I guess developer dashboard, but not being an Android dev I cannot really tell)?

That said, I've adjusted the YAML of your app here:

  • removed the AntiFeatures
  • adjusted file name matching to ApkMatch: /armeabi.*fdroid/ (so in case you want to attach the per-ABI builds of the other flavor, my updater won't accidentally pick the armeabi of that)
  • re-enabled auto-updates

Added the APK manually here as I had it already downloaded for the check, and manually triggered an update of Fastlane. So with the next sync around 7 pm UTC, all should be fine again.

Thanks a lot for all the efforts taken to reach this point! If you now want to go a step further and apply for inclusion at F-Droid.org, I'm at your service (disclosure: I'm one of the maintainers there). Should we manage the "supreme discipline" of reproducible builds there, F-Droid would ship the APKs signed by you (i.e. the very same ones I currently pick for my repo) – but with the difference of providing all ABIs then. Worth considering, right? 😃

@MagicLike
Copy link
Contributor Author

Thanks a lot for all the efforts taken to reach this point! If you now want to go a step further and apply for inclusion at F-Droid.org, I'm at your service (disclosure: I'm one of the maintainers there). Should we manage the "supreme discipline" of reproducible builds there, F-Droid would ship the APKs signed by you (i.e. the very same ones I currently pick for my repo) – but with the difference of providing all ABIs then. Worth considering, right?

I opened an issue for the inclusion into the FDroid.org repo (#646)...

@IzzySoft
Copy link

IzzySoft commented Feb 5, 2023

Just a note concerning full_description.txt:

Häufig gestellte Fragen: https://github.com/jamie-mh/AuthenticatorPro/wiki#frequently-asked-questions

You know you can use HTML there?

<a href='https://github.com/jamie-mh/AuthenticatorPro/wiki#frequently-asked-questions'>Häufig gestellte Fragen</a>

would be perfectly fine – and make that referencing link clickable.

() *Um QR-Codes zu scannen, benötigt die App die Kamera-Berechtigung.

Ugh, what went wrong there? Let's look at the original:

(*) *Um QR-Codes zu scannen, benötigt die App die Kamera-Berechtigung.*

Seems like to cause some confusion with the Markdown renderer, which recognized the first and the last asterisk, making everything between italics and then removing them 🤣 That said, especially for the F-Droid inclusion I strongly suggest to remove the empty lines inside the bullet-point list (currently lines 11,13,15…,27). F-Droid replaces each \n by <br>\n, so those empty lines will look very ugly there. It will also take the ### literally (my parser takes care for that by making the line bold; we don't want to interfere with the page structure).

MagicLike added a commit to MagicLike/AuthenticatorPro that referenced this pull request Feb 6, 2023
@MagicLike
Copy link
Contributor Author

Just a note concerning full_description.txt:

Häufig gestellte Fragen: https://github.com/jamie-mh/AuthenticatorPro/wiki#frequently-asked-questions

You know you can use HTML there?

<a href='https://github.com/jamie-mh/AuthenticatorPro/wiki#frequently-asked-questions'>Häufig gestellte Fragen</a>

would be perfectly fine – and make that referencing link clickable.

() *Um QR-Codes zu scannen, benötigt die App die Kamera-Berechtigung.

Ugh, what went wrong there? Let's look at the original:

(*) *Um QR-Codes zu scannen, benötigt die App die Kamera-Berechtigung.*

Seems like to cause some confusion with the Markdown renderer, which recognized the first and the last asterisk, making everything between italics and then removing them rofl That said, especially for the F-Droid inclusion I strongly suggest to remove the empty lines inside the bullet-point list (currently lines 11,13,15…,27). F-Droid replaces each \n by <br>\n, so those empty lines will look very ugly there. It will also take the ### literally (my parser takes care for that by making the line bold; we don't want to interfere with the page structure).

All fixed in 6822b14 & currently waiting for merge on #648 👍

@jamie-mh
Copy link
Member

jamie-mh commented Feb 6, 2023

@IzzySoft

Thanks for making the changes on your end.
Sure, you can use that graphic. It's from the Google Play developer console. Statistics may be different for other apps, but I imagine the trend is the same.

@MagicLike MagicLike mentioned this pull request Feb 6, 2023
@IzzySoft
Copy link

IzzySoft commented Feb 6, 2023

Thanks! So next time when I'm confronted with that question I've got stronger arguments 😃

I imagine the trend is the same.

"Peers median" suggests as much, yes 😉 "Peers average" would have been interesting as well; I always get headaches wrapping my mind about what median exactly is 🙈 I mean, I know – but try to explain…

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants