Skip to content

Conversation

@ligurio
Copy link
Owner

@ligurio ligurio commented Jul 3, 2025

No description provided.

Copy link
Collaborator

@Buristan Buristan left a comment

Choose a reason for hiding this comment

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

Hi, Sergey!
Thanks for the patch!
LGTM, with a minor comments below.

@Buristan Buristan assigned ligurio and unassigned Buristan Jul 17, 2025
ligurio added a commit to ligurio/lua-c-api-corpus that referenced this pull request Jul 18, 2025
ligurio added a commit to ligurio/lua-c-api-corpus that referenced this pull request Jul 18, 2025
@ligurio ligurio force-pushed the ligurio/gh-xxxx-lapi-os branch 3 times, most recently from 345a3fc to 33e1d6e Compare July 18, 2025 18:07
ligurio added a commit to ligurio/lua-c-api-corpus that referenced this pull request Jul 18, 2025
@ligurio ligurio force-pushed the ligurio/gh-xxxx-lapi-os branch 2 times, most recently from 6f144fd to 2984514 Compare July 22, 2025 12:34
@ligurio ligurio requested a review from Buristan July 22, 2025 12:34
@ligurio ligurio assigned Buristan and unassigned ligurio Jul 22, 2025
@ligurio ligurio force-pushed the ligurio/gh-xxxx-lapi-os branch from 2984514 to d8ba3bd Compare July 22, 2025 14:44
Copy link
Collaborator

@Buristan Buristan left a comment

Choose a reason for hiding this comment

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

Hi, Sergey!
Thanks for the fixes!
LGTM!

The patch adds tests for the functions in os library except the
following functions: `os.tmpname`, `os.rename`, `os.remove`,
`os.execute`, `os.clock`.
@ligurio ligurio force-pushed the ligurio/gh-xxxx-lapi-os branch from d8ba3bd to 2a06358 Compare July 24, 2025 14:24
@Buristan Buristan assigned ligurio and unassigned Buristan Jul 25, 2025
@ligurio ligurio merged commit 8606640 into master Jul 25, 2025
12 checks passed
@ligurio ligurio deleted the ligurio/gh-xxxx-lapi-os branch July 25, 2025 09:53
ligurio added a commit to ligurio/lua-c-api-corpus that referenced this pull request Jul 28, 2025
ligurio added a commit that referenced this pull request Jul 28, 2025
In some cases `os.time()` returns `nil` and this breaks the test.
The patch fixes that.

Follows up #141
ligurio added a commit that referenced this pull request Jul 28, 2025
The `mktime()` function (which is used inside `os.time()`)
returns -1 for cases when the time since the Epoch cannot be
represented. In this case, `os.time()` returns `nil`:

```
luajit -e "print(os.time({year = 1970, month = 1, day = 1, sec = -9 * 60 * 60 - 1}))"
nil
```

The patch fixes an assertion in the test and allows `nil` as
a result of the function.

Follows up #141
ligurio added a commit that referenced this pull request Jul 28, 2025
The `mktime()` function (which is used inside `os.time()`)
returns -1 for cases when the time since the Epoch cannot be
represented. In this case, `os.time()` returns `nil`:

```
luajit -e "print(os.time({year = 1970, month = 1, day = 1, sec = -9 * 60 * 60 - 1}))"
nil
```

The patch fixes an assertion in the test and allows `nil` as
a result of the function.

Follows up #141
ligurio added a commit that referenced this pull request Aug 28, 2025
The patch updates assertions for values returned by `os.date()`
and `os.time()`.

Follows up #141
ligurio added a commit that referenced this pull request Aug 29, 2025
The patch updates assertions for values returned by `os.date()`
and `os.time()`.

Follows up #141
ligurio added a commit that referenced this pull request Aug 29, 2025
The patch updates assertions for values returned by `os.date()`
and `os.time()`.

Follows up #141
ligurio added a commit that referenced this pull request Aug 29, 2025
The patch updates assertions for values returned by `os.date()`
and `os.time()`.

Follows up #141
ligurio added a commit to ligurio/lua-c-api-corpus that referenced this pull request Sep 11, 2025
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