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

Making the code more idiomatic #20

Open
wants to merge 6 commits into
base: master
Choose a base branch
from
Open

Making the code more idiomatic #20

wants to merge 6 commits into from

Conversation

jackmiras
Copy link

This is a general cleanup to make this DSL more Kotlin like. This pull request does not change the code dynamics but in some points does change the code structure in order to take advantege of the Kotlin language and to make the code shorter, cleaner and readable.

@perwendel
Copy link
Owner

I'm back from vacation and will start reviewing this very soon!

@Guppster
Copy link

Guppster commented Oct 9, 2017

@perwendel
Any update with this issue. I'd like to see the DSL further leverage Kotlin

@jackmiras
Copy link
Author

@Guppster I would like to know if this issue will be updated as well... Much has changed since I wrote the code to this pull request. At the time I was also willing to help into another issues but since all the issues is out of date I've start to wonder if worth the effort of spending time trying to solve those issues.

import kotlin.reflect.KClass

// STATIC API BEGIN
val DEFAULT_ACCEPT = "*/*"
val DEFAULT_ACCEPT by lazy { "*/*" }
Copy link

Choose a reason for hiding this comment

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

What's the purpose of making it lazy?

Copy link

Choose a reason for hiding this comment

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

More over it brings contention on getter since lazy is thread safe.

@perwendel
Copy link
Owner

I've been off track for a while for several reasons and I'm really trying to find the time to wrap up and make an official kotlin release. I know there have been changes so I will try to cherry-pick the goodies from this PR into the updated master branch.

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