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

kitty: 0.17.4 -> 0.18.0 #91189

Merged
merged 1 commit into from Jun 21, 2020
Merged

kitty: 0.17.4 -> 0.18.0 #91189

merged 1 commit into from Jun 21, 2020

Conversation

@Luflosi
Copy link
Contributor

Luflosi commented Jun 20, 2020

I submitted a patch to kitty, so that the library paths can now be passed as arguments to the build system, eliminating the need for patching. This should reduce the required maintenance effort in the future.
I removed an outdated comment.
I added a link to the changelog. Where is this link exposed to the user?
I added dontConfigure = true; to skip the configure phase since kitty has no configure script.
I now execute the test suite of kitty. Is the doCheck = true; attribute needed? It seems to work fine without but I added it anyways.
I also removed the dependency which.
Please check that the attributes are ordered in the preferred way.

Motivation for this change

https://github.com/kovidgoyal/kitty/releases/tag/v0.18.0

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.
@tex
Copy link
Contributor

tex commented Jun 20, 2020

Looks good to me. Can you squash commits into just one commit?

@teto
teto approved these changes Jun 21, 2020
Copy link
Contributor

teto left a comment

passed nixpkgs-review and binary worked fine. Maintainers can squash. Thanks for making kitty easier to maintain.

@tex
tex approved these changes Jun 21, 2020
https://github.com/kovidgoyal/kitty/releases/tag/v0.18.0

I submitted a patch to kitty, so that the library paths can now be passed as arguments to the build system, eliminating the need for patching. This should reduce the required maintenance effort in the future.

This commit removes the dependency on `which`, which is no longer necessary since 5e4fe58. It was introduced in 481e61f.

The configure phase is now ignored because kitty has no configure script.

The kitty tests are now run after compilation.

The zsh completions no longer need to be invoked with `source` since 38eb737, so remove the outdated comment.

There is now a link to the changelog.
@Luflosi Luflosi force-pushed the Luflosi:update/kitty branch from eb8e852 to f0e519f Jun 21, 2020
@Luflosi
Copy link
Contributor Author

Luflosi commented Jun 21, 2020

I put all the changes into one commit and rebased onto the latest master.

@teto teto merged commit f96feee into NixOS:master Jun 21, 2020
8 of 10 checks passed
8 of 10 checks passed
grahamcofborg-eval Beginning Evaluations
Details
grahamcofborg-eval-nixpkgs-unstable-jobset nix-instantiate --arg nixpkgs { outPath=./.; revCount=999999; shortRev="f0e519f"; rev="f0e519fa1ff4fc1e3e256a4d39320a92962eeccd"; } ./pkgs/t
Details
grahamcofborg-eval-lib-tests nix-build --arg pkgs import ./. {} ./lib/tests/release.nix
Details
grahamcofborg-eval-nixos nix-instantiate --arg nixpkgs { outPath=./.; revCount=999999; shortRev="f0e519f"; rev="f0e519fa1ff4fc1e3e256a4d39320a92962eeccd"; } ./nixos/
Details
grahamcofborg-eval-nixos-manual nix-instantiate --arg nixpkgs { outPath=./.; revCount=999999; shortRev="f0e519f"; rev="f0e519fa1ff4fc1e3e256a4d39320a92962eeccd"; } ./nixos/
Details
grahamcofborg-eval-nixos-options nix-instantiate --arg nixpkgs { outPath=./.; revCount=999999; shortRev="f0e519f"; rev="f0e519fa1ff4fc1e3e256a4d39320a92962eeccd"; } ./nixos/
Details
grahamcofborg-eval-nixpkgs-manual nix-instantiate --arg nixpkgs { outPath=./.; revCount=999999; shortRev="f0e519f"; rev="f0e519fa1ff4fc1e3e256a4d39320a92962eeccd"; } ./pkgs/t
Details
grahamcofborg-eval-nixpkgs-tarball nix-instantiate --arg nixpkgs { outPath=./.; revCount=999999; shortRev="f0e519f"; rev="f0e519fa1ff4fc1e3e256a4d39320a92962eeccd"; } ./pkgs/t
Details
grahamcofborg-eval-package-list nix-env -qa --json --file .
Details
grahamcofborg-eval-package-list-no-aliases nix-env -qa --json --file . --arg config { allowAliases = false; }
Details
@ofborg ofborg bot requested a review from tex Jun 21, 2020
@Luflosi Luflosi deleted the Luflosi:update/kitty branch Jun 21, 2020
@Luflosi
Copy link
Contributor Author

Luflosi commented Jun 21, 2020

Thanks for merging ❤️.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

3 participants
You can’t perform that action at this time.