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

feat(dialog): open buildCustomView for override #1033

Merged
merged 1 commit into from
Mar 25, 2022

Conversation

mikehardy
Copy link
Contributor

This allows complicated dynamic views in sub-classes
Related: ankidroid/Anki-Android#7805 (comment)

@F43nd1r
Copy link
Member

F43nd1r commented Mar 25, 2022

Might be better to just put an @OpenAPI on the class instead of opening specific methods

@mikehardy
Copy link
Contributor Author

I mean this literally: whatever you prefer :-), I went surgical / minimal-intervention first
I must admit I'm not familiar with @OpenAPI as an annotation, and a quick search turned nothing up for me 🤔

@F43nd1r
Copy link
Member

F43nd1r commented Mar 25, 2022

I must admit I'm not familiar with @openapi as an annotation, and a quick search turned nothing up for me 🤔

That is because it is acra-specific configuration. If you want to google what it does, you need to look for kotlin all-open, which in our case is used as gradle plugin.

Shortform is: Makes the class and all members open by default, as it would be in java.

@mikehardy
Copy link
Contributor Author

@F43nd1r oh that makes sense, then how does the combined effect of the two commits look? Is that what you were thinking?

If it was you'll either need to squash merge now or I can clean up by pulling the repo down local and rebasing - which implies that no I have not compile tested even and only now I see there's no CI here 😨 but I'll see what you think first

@@ -40,6 +41,7 @@ import org.acra.prefs.SharedPreferencesFactory
*
* @author F43nd1r & Various
*/
@OpenAPI
@Suppress("MemberVisibilityCanBePrivate")
open class CrashReportDialog : Activity(), DialogInterface.OnClickListener {
Copy link
Member

Choose a reason for hiding this comment

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

the open keyword is redundant with the annotation present

@F43nd1r
Copy link
Member

F43nd1r commented Mar 25, 2022

Huh, looks like I never enabled tests for PRs 🤔

Also note that this will most likely not make it into any 5.8 release, as I've already started 5.9 development cycle.

@mikehardy
Copy link
Contributor Author

There's also no workflow_dispatch event on them so you can't manually run them on a branch if you want :)
Looks like this if you want it: #1034

@mikehardy
Copy link
Contributor Author

Totally fine waiting for 5.9.x by the way, we've waited this long to handle the move away from annotations even! AnkiDroid is bogged down in other large projects soaking up our dev time

This allows complicated dynamic views in sub-classes
Related: ankidroid/Anki-Android#7805 (comment)
@mikehardy
Copy link
Contributor Author

Okay, no more messing around ;-)
I pulled it locally, rebased against master here, cleaned up the commits, addressed your comment, then pushed it
Action running now: https://github.com/mikehardy/acra/actions

@F43nd1r F43nd1r merged commit 35acc2c into ACRA:master Mar 25, 2022
@F43nd1r
Copy link
Member

F43nd1r commented Mar 25, 2022

Every time I think I know it'll work, something happens. I fixed it on master.

@mikehardy mikehardy deleted the patch-2 branch April 20, 2022 17:53
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