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.

Sign up for GitHub

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

Jump to bottom

Implement the timezone database for Linux and Windows in Kotlin #286

Merged
merged 2 commits into from
Dec 12, 2023
Merged

Implement the timezone database for Linux and Windows in Kotlin #286

merged 2 commits into from
Dec 12, 2023

Conversation

dkhalanskyjb
Copy link
Contributor

@dkhalanskyjb dkhalanskyjb commented Jun 26, 2023 •
edited

Fixes #262

@dkhalanskyjb dkhalanskyjb requested a review from ilya-g June 26, 2023 13:06
@dkhalanskyjb dkhalanskyjb force-pushed the pureKotlinLinux branch 3 times, most recently from 66a0bd0 to 613d49d Compare July 4, 2023 12:05
@ilya-g ilya-g mentioned this pull request Sep 8, 2023
Merged
ilya-g
ilya-g reviewed Sep 8, 2023
View reviewed changes
core/nix/test/Util.kt Outdated Show resolved Hide resolved
@dkhalanskyjb dkhalanskyjb changed the title Implement the timezone database for Linux in Kotlin Implement the timezone database for Linux and Windows in Kotlin Oct 9, 2023
@dkhalanskyjb dkhalanskyjb requested review from ilya-g and removed request for ilya-g October 9, 2023 13:10
@dkhalanskyjb dkhalanskyjb force-pushed the pureKotlinLinux branch 2 times, most recently from dafe9d7 to ac00680 Compare November 28, 2023 09:43
ilya-g
ilya-g requested changes Nov 28, 2023
View reviewed changes
core/build.gradle.kts Outdated Show resolved Hide resolved
core/common/src/DayOfWeek.kt Outdated Show resolved Hide resolved
@dkhalanskyjb dkhalanskyjb self-assigned this Nov 29, 2023
@dkhalanskyjb dkhalanskyjb added this to the 0.6.0 milestone Nov 29, 2023
ilya-g
ilya-g reviewed Nov 29, 2023
View reviewed changes
core/build.gradle.kts Outdated Show resolved Hide resolved
ilya-g
ilya-g approved these changes Dec 5, 2023
View reviewed changes
Copy link
Member

@ilya-g ilya-g left a comment

Choose a reason for hiding this comment

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

Ok, I think it's good to merge. Please squash some intermediate changes.

core/build.gradle.kts Outdated
val output = "$projectDir/native/cinterop/public/windows_zones.hpp"
val initialFileContents = File(output).readBytes()
val output = "$projectDir/windows/src/WindowsZoneNames.kt"
val initialFileContents = try { File(output).readBytes() } catch(e: Throwable) { ByteArray(0) }
Copy link
Member

Choose a reason for hiding this comment

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

Minor: is it really required to read this file at configuration phase?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

You're right, this does sometimes lead to the file being read when it doesn't need to be, e.g. when invoking gradle tasks. Good job noticing this!

@dkhalanskyjb dkhalanskyjb merged commit 79732eb into master Dec 12, 2023
1 check passed
@dkhalanskyjb dkhalanskyjb deleted the pureKotlinLinux branch December 12, 2023 09:45
@kevincianfarini kevincianfarini mentioned this pull request May 27, 2024
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ilya-g ilya-g ilya-g approved these changes

Assignees

@dkhalanskyjb dkhalanskyjb

Labels
None yet
Projects
None yet
Milestone
0.6.0
Development

Successfully merging this pull request may close these issues.

What's about Windows 7?
2 participants
@dkhalanskyjb @ilya-g