-
Notifications
You must be signed in to change notification settings - Fork 19
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
target kotlin 1.5.0 #70
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks Good To Me 🤩
"get${this.substringAfterLast(':').capitalize(Locale.ROOT)}" | ||
else -> "get${this.capitalize(Locale.ROOT)}" | ||
"get${this.substringAfterLast(':') | ||
.replaceFirstChar { if (it.isLowerCase()) it.titlecase(Locale.ROOT) else it.toString() }}" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice, and how about we wrap the following line into a function or extension? Since it was used in this file frequently?
replaceFirstChar { if (it.isLowerCase()) it.titlecase(Locale.ROOT) else it.toString() }}
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@ivanisidrowu should be fixed in latest commits let me know what you think.
btw how can I test this locally? I see no publishToMavenLocal
in ./gradlew tasks
🤔
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry, we don't have the Gradle task.
However, you can test them by running the local and instrumental tests via the project.
Tests should be found under these directories:
KtRssReader/android/src/androidTest
KtRssReader/android/src/test
KtRssReader/processorTest/src/androidTest
KtRssReader/processorTest/src/test
Click the button in the editor to run the tests.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Cool, thanks for updating the Kotlin version! :)
@@ -28,5 +29,5 @@ class KtRssReaderConfig { | |||
var flushCache: Boolean = false | |||
|
|||
@ExperimentalTime | |||
var expiredTimeMillis: Long = 1.days.toLongMilliseconds() | |||
var expiredTimeMillis: Long = Duration.days(1).inWholeMilliseconds |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
its from here the initially reported issue comes from #69 (comment)
I ran Seems like we will have to wait till turbine is built against kotlin 1.5.0 and coroutines 1.5.0. Coroutines is not yet released and is still in RC.
|
Interesting, it seems they have the same issue. I think maybe we can wait until they release the new version. Then, we can merge this PR. |
@ivanisidrowu its out -- I bumped coroutines, turbine and room while at it. But still I don't know if this solves the issue I originally mentioned as I cannot test this in my own app. I think it would be a good idea to add the |
Hi @sphrak, sounds good, I will open a task to do it as an enhancement. In the mean time, I think you might be able to test it with JitPack. So you can just include these libs to test in your own app:
If you would like to test it by yourself, you can go to: https://jitpack.io/#sphrak/KtRssReader/target-kotlin-1.5.0-SNAPSHOT Hope it helps. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good, it's ready to merge.
@ivanisidrowu thank you very much 😺 I just tested the snapshot you published and I can no longer see the crash so I think you can release a stable version 👍 it works™ |
@sphrak No problem! It's always great to see people are using our lib. :) I will release v2.1.2 today. If you have suggestions in the future, don't hesitate to let us know. Happy coding! |
No description provided.