Skip to content

Conversation

mikeland73
Copy link
Contributor

Summary

A few improvements:

  • Use same refresh command. Previously we were using two different ways of refreshing the environment.
  • Display refresh-global even if alias is not set (since it will likely be set by the end of the command).
  • Don't show the refresh message will calling refresh.

How was it tested?

  • Tested refresh-global on zsh.
  • TODO: test refresh-global on fish.
  • Calling refresh-global does not show refresh message.

@mikeland73 mikeland73 requested review from gcurtis and savil June 26, 2024 00:43
Copy link
Collaborator

@savil savil left a comment

Choose a reason for hiding this comment

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

thanks!

@@ -37,6 +37,8 @@ import (
"go.jetpack.io/devbox/internal/ux"
)

var StateOutOfDateMessage = "Your devbox environment may be out of date. Run %s to update it.\n"
Copy link
Collaborator

Choose a reason for hiding this comment

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

const?

@@ -285,9 +287,10 @@ func (d *Devbox) ensureStateIsUpToDate(ctx context.Context, mode installMode) er
// be out of date after the user installs something. If have direnv active
// it should reload automatically so we don't need to refresh.
if d.IsEnvEnabled() && !upToDate && !d.IsDirenvActive() {
ux.Fwarning(
ux.FHidableWarning(
Copy link
Collaborator

Choose a reason for hiding this comment

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

TIL about FHidableWarning

@mikeland73 mikeland73 merged commit 7d7741b into main Jun 28, 2024
@mikeland73 mikeland73 deleted the landau/improve-refresh branch June 28, 2024 03:48
Copy link

sentry-io bot commented Jun 28, 2024

Suspect Issues

This pull request was deployed and Sentry observed the following issues:

  • ‼️ **syscall.Errno: <redacted errors.withStack>: <redacted fs.PathError>: go.jetpack.io/devbox/internal/nix in (*Nix).Pri... View Issue

Did you find this useful? React with a 👍 or 👎

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.

2 participants