-
Notifications
You must be signed in to change notification settings - Fork 7
Implement importFlake #430
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
Conversation
a0e1c14 to
1ded956
Compare
|
Updated per discussion above. Let me know what you think! This is still in draft since I haven't pulled anything except the fileserver and checks. |
0686c28 to
304fdfb
Compare
soenkehahn
left a comment
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.
We need a changelog entry.
ts/importFlake.ts
Outdated
| import { mkPackage, Package } from "./package.ts"; | ||
|
|
||
| function getFlake(flakeDir: string): NixExpression { | ||
| if (flakeDir.match(/^[.][.]?[/]/)) { |
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.
This matches relative and absolute paths?
Btw, there's also lots of other flake url syntax options, see here and here lists all possible types. So e.g. file:... and path:... are valid flake urls. I don't think we have to support any of this besides what's currently implemented. But 1. I wanted you to know those exist and 2. maybe we should think about what happens with those right now.
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.
This matches relative paths only at the moment. Do you think we should support absolute paths? One thing I realized in testing is that the only thing that would break when using a flake input vs callFlake is parent path imports. Maybe we can experiment with file:... and path:... tomorrow and see how they interact with both flake inputs and callFlake
|
Forgot to mention that I'm very excited about this PR! :) |
16f5eca to
61a979f
Compare
soenkehahn
left a comment
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.
LGTM.
Short POC that implements #427 and is a step toward completing #348. Just threw this together to open a discussion to see if this is the direction we were all thinking about for #427, or if we wanted something different.