Skip to content
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

Kubecolor oc alias and zsh completions #6627

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

jennydaman
Copy link

@jennydaman jennydaman commented Mar 14, 2025

Description

Checklist

  • Change is backwards compatible.

  • Code formatted with ./format.

  • Code tested through nix-shell --pure tests -A run.all
    or nix build --reference-lock-file flake.lock ./tests#test-all using Flakes.

  • Test cases updated/added. See example.

  • Commit messages are formatted like

    {component}: {description}
    
    {long description}
    

    See CONTRIBUTING for more information and recent commit messages for examples.

  • If this PR adds a new module

    • Added myself as module maintainer. See example.

Maintainer CC

@ajgon

@ajgon
Copy link
Contributor

ajgon commented Mar 14, 2025

I think your commits got broken somehow, and you've lost the oc alias part :-).

@jennydaman
Copy link
Author

Whoops, you're right. I made a mistake rewriting history to run ./format.

@jennydaman
Copy link
Author

@ajgon ptal?

Comment on lines 95 to 98
oc = "env KUBECTL_COMMAND=${lib.getExe pkgs.openshift} ${
lib.getExe cfg.package
}";
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
oc = "env KUBECTL_COMMAND=${lib.getExe pkgs.openshift} ${
lib.getExe cfg.package
}";
oc = lib.mkIf (lib.elem pkgs.openshift config.home-manager.users.ajgon.home.packages || lib.elem pkgs.openshift config.environment.systemPackages) "env KUBECTL_COMMAND=${lib.getExe pkgs.openshift} ${
lib.getExe cfg.package
}";

I would add this alias only if user has openshift installed.

version = "5.9";
};
};
home.packages = [ pkgs.openshift ];
Copy link
Author

Choose a reason for hiding this comment

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

@ajgon I added some tests for the conditional addition of the oc alias.

This is my first PR to nix-community. I'm not sure what the best way to test this would be. Is it bad to have pkgs.openshift in the test here, instead of config.lib.test.mkStubPackage?

Copy link
Contributor

Choose a reason for hiding this comment

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

I think mkStubPackage will work better here, as it won't build whole derivation for openshift, just for testing. You can overload pkgs with overlay i.e.

  nixpkgs.overlays = [
    (self: super: rec {
      openshift = config.lib.test.mkStubPackage {
        name = "openshift";
        version = "4.16.0";
    };
  ];

and keep the rest of your test as is. This should use stub instead of a real package, thus avoiding building deriv. 'm not at my laptop right now, so can't guarantee if it works, but in general that's the idea you should follow :-).

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.

2 participants