-
-
Notifications
You must be signed in to change notification settings - Fork 1.5k
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
Ensure all store types support "real" URIs #9850
Conversation
8b238ac
to
cbf479e
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice :)
I'm not really convinced by the switch to an std::optional
for the authority parameter (we nearly always convert it straight away to an empty string, so we just end-up with std::optional{""} == std::nullopt
), but the extra consistency for the rest is very nice
31a1cce
to
17b0a12
Compare
17b0a12
to
d95eedd
Compare
Got rid of the |
In particular `local://<path>` and `unix://` (without any path) now work, and mean the same things as `local` and `daemon`, respectively. We thus now have the opportunity to desguar `local` and `daemon` early. This will allow me to make a change to NixOS#9839 requested during review to desugar those earlier. Co-authored-by: Théophane Hufschmitt <[email protected]>
d95eedd
to
470c050
Compare
Motivation
In particular
local://<path>
andunix://
(without any path) now work, and mean the same things aslocal
anddaemon
, respectively. We thus now have the opportunity to desguarlocal
anddaemon
early.Context
This will allow me to make a change to #9839 requested during review to desugar those earlier.
Priorities and Process
Add 👍 to pull requests you find important.
The Nix maintainer team uses a GitHub project board to schedule and track reviews.