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

replacement: init at 0.4.4 #95210

Merged
merged 1 commit into from Aug 22, 2020
Merged

Conversation

siriobalmelli
Copy link
Contributor

Signed-off-by: Sirio Balmelli sirio@b-ad.ch

Motivation for this change

Add new python package for use in a Nix CI/CD

Things done
  • Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS linux)
  • Built on platform(s)
    • NixOS
    • macOS
    • other Linux distributions
  • Tested via one or more NixOS test(s) if existing and applicable for the change (look inside nixos/tests)
  • Tested compilation of all pkgs that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review wip"
  • Tested execution of all binary files (usually in ./result/bin/)
  • Determined the impact on package closure size (by running nix path-info -S before and after)
  • Ensured that relevant documentation is up to date
  • Fits CONTRIBUTING.md.

pkgs/development/tools/misc/replacement/default.nix Outdated Show resolved Hide resolved
pkgs/development/tools/misc/replacement/default.nix Outdated Show resolved Hide resolved
pkgs/top-level/all-packages.nix Outdated Show resolved Hide resolved
pkgs/development/tools/misc/replacement/default.nix Outdated Show resolved Hide resolved
pkgs/development/tools/misc/replacement/default.nix Outdated Show resolved Hide resolved
pkgs/development/tools/misc/replacement/default.nix Outdated Show resolved Hide resolved
@doronbehar
Copy link
Contributor

Note that CI fails.

Copy link
Contributor

@doronbehar doronbehar left a comment

Choose a reason for hiding this comment

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

Untested suggestions.

pkgs/development/tools/misc/replacement/default.nix Outdated Show resolved Hide resolved
pkgs/development/tools/misc/replacement/default.nix Outdated Show resolved Hide resolved
pkgs/development/tools/misc/replacement/default.nix Outdated Show resolved Hide resolved
pkgs/development/tools/misc/replacement/default.nix Outdated Show resolved Hide resolved
pkgs/development/tools/misc/replacement/default.nix Outdated Show resolved Hide resolved
@doronbehar
Copy link
Contributor

@siriobalmelli You did not follow my requests. Please read the link I gave you to nix.dev, and please don't use with python.pkgs for such a broad scope.

@siriobalmelli
Copy link
Contributor Author

@siriobalmelli You did not follow my requests. Please read the link I gave you to nix.dev, and please don't use with python.pkgs for such a broad scope.

My apologies, I messed that one up.

Force-pushed with a specific use of python3Packages. or with python3Packages; in each case.

Copy link
Contributor

@doronbehar doronbehar left a comment

Choose a reason for hiding this comment

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

Passes nixpkgs-review build and --help test.

Signed-off-by: Sirio Balmelli <sirio@b-ad.ch>
Co-Authored-By: Doron Behar <doron.behar@gmail.com>
@infinisil
Copy link
Member

It is a bit questionable whether we should include this package, see https://discourse.nixos.org/t/we-need-more-defined-guidelines-for-package-inclusion/3592. Is anybody other than you using this @siriobalmelli? Alternatively you could also use NUR to distribute the package.

@siriobalmelli
Copy link
Contributor Author

It is a bit questionable whether we should include this package, see https://discourse.nixos.org/t/we-need-more-defined-guidelines-for-package-inclusion/3592. Is anybody other than you using this @siriobalmelli? Alternatively you could also use NUR to distribute the package.

I see what you mean.

This particular package has been around since 2018 and is in use downstream of me. You will see in the project history that a nix derivation was first added in day 2 of development.

Part of the motivation in trying to get it accepted into nixpkgs is so those downstreams don't have to always point to https://github.com/siriobalmelli-foss/nixpkgs or use a custom import like:

  replacement = import (builtins.fetchGit {
    url = "https://siriobalmelli@github.com/siriobalmelli/replacement.git";
    ref = "master";
    }) {};

Re: maintenance I don't foresee a problem - Nix is in heavy use in our shop, I am a regular contributor to nixpkgs and I'm signing off as the maintainer of this package: I can always be pinged if it becomes a problem later.

In getting adoption for Nix with other engineers, it would be of enormous aid having them be able to install Nix as per standard and then just run nix-env -iA "nixpkgs.replacement"

Based on the above, please consider merging this: it makes it that much more productive to contribute to nixpkgs in my own time if I can leverage the same nixpkgs to improve efficiency on other projects :)

@infinisil
Copy link
Member

Sounds good then!

@infinisil infinisil merged commit f74bdc5 into NixOS:master Aug 22, 2020
@siriobalmelli siriobalmelli deleted the add/replacement branch August 22, 2020 17:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants