Skip to content

Conversation

@davepacheco
Copy link
Collaborator

@davepacheco davepacheco commented May 4, 2023

I like the new tools/setup_env.sh. Here I'm taking a few steps to first-class it a bit more:

  • it should now work to add the various binaries to your PATH regardless of where you run it from (i.e., you don't have to run it from the root of the workspace)
  • the CI scripts use it rather than hardcoding the same paths, which hopefully will help make sure this stays up to date
  • I moved it to the root and called it env.sh, only because I've seen this convention before. If others have other preferences I'm happy to leave it as it is or call it something else.

@davepacheco
Copy link
Collaborator Author

One test run seems to have hit #3005: https://github.com/oxidecomputer/omicron/pull/3006/checks?check_run_id=13220424276
I'm going to rerun it.

@davepacheco davepacheco marked this pull request as ready for review May 4, 2023 14:37
Copy link
Contributor

@internet-diglett internet-diglett left a comment

Choose a reason for hiding this comment

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

LGTM, just one comment

Comment on lines 33 to 39
Now put these on your PATH with:

[source,text]
----
$ source env.sh
----

Copy link
Contributor

Choose a reason for hiding this comment

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

We might want to advise running this before installing prerequisites, as I think the script will bail out early if the path isn't already set up?

@davepacheco davepacheco enabled auto-merge (squash) May 4, 2023 17:33
@davepacheco davepacheco merged commit 7e8273e into main May 4, 2023
@davepacheco davepacheco deleted the dap/env branch May 4, 2023 19:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants