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: Add Jodel support (#462) #463

Merged
merged 3 commits into from
Sep 18, 2024
Merged

Conversation

xyrolaith
Copy link
Contributor

@xyrolaith xyrolaith commented Sep 17, 2024

I have never done any Kotlin programming before so please double check.
Also the invoke function could probably use some extra validations.

Fixes #462

@svenjacobs
Copy link
Owner

Thanks, this looks really good for your first Kotlin code 🙌🏼

Please run ./gradlew formatKotlin once to format the code according to our formatting rules. Thanks.

@xyrolaith
Copy link
Contributor Author

The formatting seems to be ok now but the test throws an error as the JSON part requires additional mocking which I am unfamiliar with.

@svenjacobs
Copy link
Owner

I will have a look 👀

@svenjacobs
Copy link
Owner

JSONObject is from the Android framework and thus is missing an implementation in unit tests, because unit tests don't run on Android devices. Mocking JSONObject doesn't make sense because then the whole test doesn't make sense. We would test a mocked object. So I'm using KotlinX Serialization now to parse the JSON.

@svenjacobs svenjacobs merged commit 42f2ca2 into svenjacobs:main Sep 18, 2024
1 check passed
@svenjacobs
Copy link
Owner

@all-contributors please add @xyrolaith for code

Copy link
Contributor

@svenjacobs

I've put up a pull request to add @xyrolaith! 🎉

@svenjacobs
Copy link
Owner

@all-contributors please add @xyrolaith for ideas

Copy link
Contributor

@svenjacobs

I've put up a pull request to add @xyrolaith! 🎉

@svenjacobs svenjacobs added the feature New feature or request label Sep 18, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Feature Request] Jodel social app
2 participants