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

Changed signature for openat function #2139

Merged
merged 5 commits into from
Oct 1, 2023

Conversation

carlosb1
Copy link
Contributor

@carlosb1 carlosb1 commented Sep 29, 2023

Fixing #1850 , Changed signature to use an Optional rawfd

Signed-off-by: carlosb1 <mcflurry0@gmail.com>
@carlosb1 carlosb1 changed the title Changed signature for openat function WIP: Changed signature for openat function Sep 29, 2023
Signed-off-by: carlosb1 <mcflurry0@gmail.com>
@carlosb1 carlosb1 changed the title WIP: Changed signature for openat function Changed signature for openat function Sep 29, 2023
@SteveLauC
Copy link
Member

SteveLauC commented Sep 30, 2023

PR LGTM, except for the missing change log.

You need to add a change log to CHANGELOG.md, it would be something like this:

## [Unreleased] - ReleaseDate

### Changed

- xxx

- `openat()` and `Dir::openat()` now take optional `dirfd`s
  ([#2134](https://github.com/nix-rust/nix/pull/2139))

And, it would be great if you squash your commits into a single one :)


Update: commits don't need to be squashed as this will be done automatically

Signed-off-by: carlosb1 <mcflurry0@gmail.com>
Signed-off-by: carlosb1 <mcflurry0@gmail.com>
@carlosb1
Copy link
Contributor Author

I updated the changelog as you commented. I expect now it is correct

CHANGELOG.md Outdated Show resolved Hide resolved
@SteveLauC SteveLauC self-assigned this Oct 1, 2023
@carlosb1
Copy link
Contributor Author

carlosb1 commented Oct 1, 2023

I can not merge it, I don't have write access, very happy to contribute in anything else.

@SteveLauC SteveLauC added this pull request to the merge queue Oct 1, 2023
@SteveLauC
Copy link
Member

Thanks for doing this! :)

Merged via the queue into nix-rust:master with commit 489621e Oct 1, 2023
35 checks passed
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.

2 participants