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

document the way to use gendex and dex.go for new GoNativeActivity.java behaviour #5387

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

dolanor
Copy link

@dolanor dolanor commented Jan 9, 2025

No description provided.

Copy link
Member

@Jacalz Jacalz left a comment

Choose a reason for hiding this comment

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

FYI: We are planning on deprecating this tool here in favour of moving it into https://github.com/fyne-io/tools. I think I changed how the generation is done in that project.

Copy link
Member

@andydotxyz andydotxyz left a comment

Choose a reason for hiding this comment

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

Great additions thanks, but I think one of the docs is a bit misleading.

@@ -27,6 +27,11 @@
import android.widget.TextView;
import android.widget.TextView.OnEditorActionListener;

// GoNativeActivity is the java implementation that helps Go map to android via the NDK.
Copy link
Member

Choose a reason for hiding this comment

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

I'm not sure this first sentence is right - the activity is the entry point for our Android boot loader.
The NDK connects to this via C - so it's not really providing any connections itself I don't think?...

Copy link
Author

Choose a reason for hiding this comment

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

What I meant (from my VERY limited understanding) is that it was the java SDK object that was a utility for Go to access via the C and the NDK.
I guess I don't know how to express it correctly :)

Copy link
Member

Choose a reason for hiding this comment

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

Go accesses the Java object via NDK. The Java provides us certain functionality which is accessed through JNI and C functionality.
Is that clearer?

@dolanor
Copy link
Author

dolanor commented Jan 9, 2025

Ok.

What's the new procedure to generate a new dex, then?
I kind of need it

@andydotxyz
Copy link
Member

Ok.

What's the new procedure to generate a new dex, then? I kind of need it

We are in the process of moving cmd/fyne into the tools repo, but nothing else changes. So this could/should land as I believe the tools repo is pending a merge as things have got behind again. Is that about right @sdassow ?

@coveralls
Copy link

Coverage Status

coverage: 60.01% (-0.005%) from 60.015%
when pulling a3cfe7b on dolanor:master
into 4a875d9 on fyne-io:master.

@sdassow
Copy link
Contributor

sdassow commented Jan 12, 2025

Yes, please. I've just done a merge, so we should be able to move ahead without too much delay: fyne-io/tools#24

@sdassow
Copy link
Contributor

sdassow commented Jan 12, 2025

Can you please file PR against the Fyne tools repo at https://github.com/fyne-io/tools? That way you can also just file against the main branch there.

@Jacalz
Copy link
Member

Jacalz commented Jan 12, 2025

That reminds me, please note that PRs should go to the develop branch in this repository and that the PR template should be filled out accordingly, not removed.

@andydotxyz
Copy link
Member

Thanks @Jacalz I missed that!

@dolanor
Copy link
Author

dolanor commented Jan 12, 2025

So the fyne CLI will be build from fyne-io/tools.

So it's gonna base on the GoNativeActivity that would be referenced from the fyne.io/fyne/v2 dependency in the go.mod, right?
So if we want to integrate updates from a modified GoNativeActivity, we would need to add some replace in the tools repo, right?

@Jacalz
Copy link
Member

Jacalz commented Jan 13, 2025

I already implementing the support for updating the Dex stuff in that repository. It relies on the cloned fyne and tools repositories being in the same parent folder as it just does a relative lookup. No need to replace anything in the module.

@andydotxyz
Copy link
Member

Yes it should work the same way, though perhaps we need to document the additional assumption that "tools" and "fyne" repos are checked out next to each other.

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.

5 participants