Upgrade downstreamPlaceholder to a type with methods#8353
Upgrade downstreamPlaceholder to a type with methods#8353roberth merged 2 commits intoNixOS:masterfrom
downstreamPlaceholder to a type with methods#8353Conversation
Ericson2314
force-pushed
the
downstream-installable-type
branch
from
b279a0f to
d0d7b49
Compare
|
OK think I addressed everything. |
roberth
left a comment
roberth
left a comment
There was a problem hiding this comment.
C++ lgtm. My suggestions to use both file static and class static were incoherent, but you resolved that well.
I assume we will later have a use for this type, and not just the construct + render combination?
Also just to be sure, what's the stability status of these hashes? I would hope they are meant to be stable but a change (if we were to accidentally make one) would rarely be observed by users? Or am I way off? Wouldn't nix eval print them? Is this documented somewhere? Do we have a test that checks the placeholder against a hardcoded example to make sure it doesn't change by accident?
Yes, once
Yes they will end up in derivations (in memory and drv files, so they shouldn't change). If they do change, trying to build old ca or dynamic-derivation-depending derivations could fail because they no longer get substituted. We could write some tests, certainly. Don't think there are interesting properties to test, but we could just put in a fixed case of each. (Similarly we should pin down the hash-modulo stuff at some point too.) |
This is good in general, but in particular ensures when we heavily refactor it in the next commit there is less likelihood for an unintentional change in behavior to sneak in.
This gets us ready for dynamic derivation dependencies (part of RFC 92).
Ericson2314
force-pushed
the
downstream-installable-type
branch
from
d0d7b49 to
b9e5ce4
Compare
github-actions
bot
added
the
with-tests
|
I added some tests. The first I added test prior to the refactor so we can see that behavior is preserved. |
2 participants
Motivation
This gets us ready for dynamic derivation dependencies (part of RFC 92).
Context
Tracking issue for RFC 92 #6316
Checklist for maintainers
Maintainers: tick if completed or explain if not relevant
tests/**.shsrc/*/teststests/nixos/*Priorities
Add 👍 to pull requests you find important.