Skip to content

Conversation

@DanAlbert
Copy link
Member

No description provided.

@DanAlbert DanAlbert force-pushed the JNI_OnLoad branch 2 times, most recently from 3cf49f4 to 759fa23 Compare September 18, 2025 23:08
@enh-google
Copy link
Contributor

at least for the cases where this is adding more lines of boilerplate than there is of actual code, is there some motivation we can add to the commit message?

@DanAlbert
Copy link
Member Author

at least for the cases where this is adding more lines of boilerplate than there is of actual code, is there some motivation we can add to the commit message?

There are 18 commit messages, but I suppose I can go edit all of them. Alternatively, I've been thinking that I should probably add a doc (maybe just in the readme) that explains some of the best practices we're aiming to show. I can add another commit that adds that language instead, though I may do that in a separate PR if that's alright, since I suspect I'll need some feedback from you to nail down exactly what to say about this particular practice. I'm not an expert at JNI, but I am apparently an expert at internalizing the message of your rants without always remembering your reasons 😆

@DanAlbert
Copy link
Member Author

at least for the cases where this is adding more lines of boilerplate than there is of actual code, is there some motivation we can add to the commit message?

Alternatively, I've been thinking that I should probably add a doc (maybe just in the readme) that explains some of the best practices we're aiming to show. I can add another commit that adds that language instead, though I may do that in a separate PR if that's alright, since I suspect I'll need some feedback from you to nail down exactly what to say about this particular practice. I'm not an expert at JNI, but I am apparently an expert at internalizing the message of your rants without always remembering your reasons 😆

Done #1153

@enh-google
Copy link
Contributor

(approved anyway since this isn't wrong, even if it's not what i'd do [except maybe for the more complicated examples]...)

@DanAlbert
Copy link
Member Author

(approved anyway since this isn't wrong, even if it's not what i'd do [except maybe for the more complicated examples]...)

Oh, and there's the answer to my question earlier. I'd overlooked this comment :)

@DanAlbert DanAlbert merged commit a8cd972 into android:main Sep 22, 2025
2 checks passed
@DanAlbert DanAlbert deleted the JNI_OnLoad branch September 22, 2025 20:28
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