Skip to content

Convert achievements models to Kotlin #3096

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

Closed
wants to merge 1 commit into from
Closed

Convert achievements models to Kotlin #3096

wants to merge 1 commit into from

Conversation

veyndan
Copy link
Contributor

@veyndan veyndan commented Jul 28, 2019

This relates to #747.

This is a demonstration (and hopefully a first step) on how Kotlin can be used in the main application. I have chosen to convert a subset of models as these are fairly ease to convert over, shows the extent of Kotlin's conciseness (250 lines down to 50 lines), and adds Kotlin's niceties when using data classes (like automatically generating toString(), equals(), and hashcode() methods).

Copy link

@tests-checker tests-checker bot left a comment

Choose a reason for hiding this comment

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

Could you please add tests to make sure this change works as expected?

@misaochan
Copy link
Member

Welcome back, @veyndan ! :)

I don't think we have reached a consensus on whether or not we want to convert everything (besides tests) to Kotlin. Would anyone else like to weigh in on this? This is a big decision IMO, not something to be done lightly. While Kotlin can certainly coexist with Java code, it would be very messy to have patches of it if we don't plan on eventually converting everything.

@veyndan
Copy link
Contributor Author

veyndan commented Jul 30, 2019

Thanks! Had to take a bit of a hiatus due to other commitments.

A further discussion around this would be beneficial as the adoption of Kotlin in the Android community has greatly increased since then, so Kotlin being a barrier to entry is now less of a concern. This PR shows some concrete usage of Kotlin in the project, and will hopefully revitalise the previous discussion.

Btw, I opened #2943 a while ago relating to Kotlin in tests if anyone has time to look at it.

@maskaravivek
Copy link
Member

@veyndan If you could fix the conflicts, I would be happy to merge this PR. Now we have adopted Kotlin in our main codebase. :)

@madhurgupta10
Copy link
Collaborator

@veyndan Are you still working on this PR, I am converting java files to kotlin, I can add these changes in my PR #3439 :)

@macgills
Copy link
Contributor

These files have been converted but could use improvement. I am going to make an issue for that but will be closing this PR

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