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

Kotlin revolution #401

Merged
merged 9 commits into from
Dec 27, 2022
Merged

Kotlin revolution #401

merged 9 commits into from
Dec 27, 2022

Conversation

ImUrX
Copy link
Member

@ImUrX ImUrX commented Dec 16, 2022

No description provided.

@ButterscotchV ButterscotchV added the Difficulty: Large refactor Requires modifying large sections of the codebase label Dec 17, 2022
@TheButlah
Copy link
Contributor

TheButlah commented Dec 17, 2022

I don't think we want the src/main/java vs src/main/kotlin style. I'd like it to be .kt and .java files in the same folders preferably. The reason for this is:

  • Java and kotlin can be in the same library, so why dupe the folder structure
  • The changes are intentionally to be incremental so renaming the file seems appropriate
  • I generally dislike the src/main/java/me/thebutlah/whatever style, it seems to be very inflexible with multi languages as it is.

I was also hoping we would keep VRServer intact for now and make a tiny almost insignificant change at first, which is just the tooling...

@ImUrX
Copy link
Member Author

ImUrX commented Dec 17, 2022

Yeah I'm not against the first part.

I didnt touch VRServer tho, only touched Main in purpose to give an example Kotlin file

@ImUrX
Copy link
Member Author

ImUrX commented Dec 18, 2022

how is this?

@ImUrX ImUrX requested a review from Eirenliel December 19, 2022 16:21
@ImUrX ImUrX force-pushed the kotlin-revolution branch 2 times, most recently from 279a831 to a9c293b Compare December 22, 2022 03:30
@TheButlah
Copy link
Contributor

TheButlah commented Dec 22, 2022

This looks good to me.

As an aside: I know things like syntax don't truly matter. But I look at this code and the val my_var: Type = asdf where the type can be omitted, is really nice to look at :)

@Eirenliel Eirenliel merged commit 5dd5399 into SlimeVR:main Dec 27, 2022
@ImUrX ImUrX deleted the kotlin-revolution branch January 13, 2023 06:02
@ImUrX
Copy link
Member Author

ImUrX commented Jan 20, 2023

Related to #398

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Difficulty: Large refactor Requires modifying large sections of the codebase
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants