-
Notifications
You must be signed in to change notification settings - Fork 1.3k
Conversation
877f1ae
to
b809970
Compare
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.
Approving, because the little suggestion I made is pretty straightforward and it would be okay to merge as is any way, as it's mostly cosmetic :)
@@ -74,19 +74,50 @@ http_archive( | |||
urls = ["https://github.com/bazelbuild/rules_rust/releases/download/0.19.0/rules_rust-v0.19.0.tar.gz"], | |||
) | |||
|
|||
http_archive( |
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.
Can we move this into dev/nix.bzl
?
You can basically take all of this code, and wrap it under a macro:
# dev/nix.bzl
def nix_deps():
http_archive(...)
# WORKSPACE
load("//dev:nix.bzl", "nix_deps")
nix_deps()
This way we avoid lengthening this file further 😊
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.
Done as best I could here, it seems like rules_nixpkgs_dependencies
must be called before any of the other symbols are loaded, so some stuff remains in WORKSPACE
b809970
to
c92076f
Compare
Will need to rebase this now that https://github.com/sourcegraph/sourcegraph/pull/49884 landed, job for tomorrow |
c92076f
to
916f03f
Compare
Some sprinkling of special sauce for the NixOS (and nix shell) users in the house. Much care has been taken so that there should be no interference with anyone running bazel on a non-nixos machine, and that it also works as (mostly) expected for nix shell users on macos.
Test plan
Ran main dry-run, all green, so it shouldnt interfere with anyone on a non-nixos machine