Skip to content

Conversation

@tkob
Copy link
Contributor

@tkob tkob commented Aug 22, 2015

This PR adds mlr_timegm function and replace timegm with mlr_timegm. (Issue #20)

johnkerl added a commit that referenced this pull request Aug 22, 2015
@johnkerl johnkerl merged commit fa9b374 into johnkerl:master Aug 22, 2015
@johnkerl
Copy link
Owner

Nice, thank you!

@tkob tkob deleted the replace-timegm branch September 5, 2015 13:16
@jirutka
Copy link

jirutka commented Jun 13, 2025

This is a quite bad improvement – mlr_timegm is not thread-safe!

@johnkerl
Copy link
Owner

@jirutka can you tell me how threads are involved in Miller?

@jirutka
Copy link

jirutka commented Jun 13, 2025

I don’t know. I was just searching for a portable timegm replacement when I came across this PR. Even if you currently don’t use threads in the project, writing code like this without any warning/comment that it’s not thread-safe is generally a pretty bad practice.

@johnkerl
Copy link
Owner

@jirutka good call!

@johnkerl
Copy link
Owner

@jirutka Miller was ported from C to Go as of 2022, and the C code has been removed from the main branch of Miller. I'll add a comment on a 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.

3 participants