Skip to content

Conversation

ipince
Copy link
Contributor

@ipince ipince commented Feb 7, 2023

Summary

This mainly refactors the env computation from nix.Run() into a place where it can be reused for both shell, run, and direnv integration. This should be a functional no-op.

How was it tested?

./devbox run -- echo '$PATH' and other env vars

@ipince ipince requested a review from savil February 7, 2023 17:16
return nil
}

func (d *Devbox) computeNixEnv() ([]string, error) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Should this be called in other places we need env? (Shell and RunScriptInNewNixShell)

Copy link
Collaborator

Choose a reason for hiding this comment

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

I believe that is the intention (in a future PR)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Correct!

return nil
}

func (d *Devbox) computeNixEnv() ([]string, error) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

I believe that is the intention (in a future PR)


func (d *Devbox) computeNixEnv() ([]string, error) {
nixShellFilePath := filepath.Join(d.projectDir, ".devbox/gen/shell.nix")
vaf, err := nix.PrintDevEnv(nixShellFilePath)
Copy link
Collaborator

Choose a reason for hiding this comment

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

use: d.nixShellFilePath() but first please approve #584 so it can be landed :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

oops, sorry for not seeing that earlier! merging in main and updating...

@ipince ipince merged commit cf32331 into main Feb 9, 2023
@ipince ipince deleted the rodrigo/compute-env branch February 9, 2023 16:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants