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

Refactor unix domain socket store config #11109

Merged
merged 27 commits into from
Jul 18, 2024

Conversation

fzakaria
Copy link
Contributor

Motivation

Following what is outlined in #10766 refactor the uds-remote-store such that the member variables (state) don't live in the store itself but in the config object.

Additionally, the config object includes a new necessary constructor that takes a scheme & authority.

Minor:

  • code formatting
  • cleanup of getting default path
  • added some comments

Context

Please see #10766
This adds part of the work for the unix domain socket store.

Priorities and Process

Add 👍 to pull requests you find important.

The Nix maintainer team uses a GitHub project board to schedule and track reviews.

Following what is outlined in NixOS#10766 refactor the uds-remote-store such
that the member variables (state) don't live in the store itself but in
the config object.

Additionally, the config object includes a new necessary constructor
that takes a scheme & authority.

Minor:
* code formatting
* cleanup of getting default path
* added some comments
@fzakaria fzakaria requested a review from Ericson2314 as a code owner July 15, 2024 18:12
@github-actions github-actions bot added the store Issues and pull requests concerning the Nix store label Jul 15, 2024
@fzakaria fzakaria requested a review from edolstra as a code owner July 15, 2024 18:42
src/libstore/uds-remote-store.hh Outdated Show resolved Hide resolved
src/libstore/uds-remote-store.hh Outdated Show resolved Hide resolved
src/libstore/uds-remote-store.hh Outdated Show resolved Hide resolved
fzakaria and others added 2 commits July 15, 2024 13:03
@fzakaria fzakaria requested a review from Ericson2314 July 15, 2024 20:09
Copy link
Member

@Ericson2314 Ericson2314 left a comment

Choose a reason for hiding this comment

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

It would be good to have a unit test like #11108, very close otherwise! :)

fzakaria and others added 3 commits July 15, 2024 14:15
Revert back to basic constructor in store-api.cc

Co-authored-by: John Ericson <git@JohnEricson.me>
@fzakaria fzakaria requested a review from Ericson2314 July 15, 2024 21:33
@fzakaria fzakaria requested a review from Ericson2314 July 16, 2024 01:14
@fzakaria
Copy link
Contributor Author

Failing MacOS builds but not sure why -- appreciate any pointer.

@Ericson2314 Ericson2314 enabled auto-merge July 16, 2024 03:18
@Ericson2314 Ericson2314 disabled auto-merge July 16, 2024 03:35
@Ericson2314 Ericson2314 enabled auto-merge (squash) July 16, 2024 03:36
@Ericson2314 Ericson2314 changed the title feat: Refactor unix domain socket store config Refactor unix domain socket store config Jul 16, 2024
auto-merge was automatically disabled July 16, 2024 12:47

Head branch was pushed to by a user without write access

@Ericson2314 Ericson2314 merged commit 57399bf into NixOS:master Jul 18, 2024
11 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
store Issues and pull requests concerning the Nix store
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants