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

iosevka, iosevka-bin: 2.3.3 -> 3.2.2 #88533

Merged
merged 2 commits into from Jul 7, 2020
Merged

Conversation

@rileyinman
Copy link
Contributor

rileyinman commented May 21, 2020

Motivation for this change

Iosevka has a new stable release! The build process has remained the same, but several options (such as Iosevka Term being renamed to Iosevka Fixed, as well as individual character style names) have been changed. This may break some custom builds, so I'm not sure if there should be a warning or if we should leave it up to users to update their build options.

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

asymmetric left a comment

I think this should be moved to node-packages-v12.json, since Iosevka now requires Node > 12.

For the same reason, we might need

  iosevka = callPackage ../data/fonts/iosevka {
    nodejs = nodejs-12_x;
    nodePackages = nodePackages_12_x;
  };

in all-packages.nix.

pkgs/data/fonts/iosevka/package.json Outdated Show resolved Hide resolved
@rileyinman
Copy link
Contributor Author

rileyinman commented May 24, 2020

Good catch, I'll rebuild it after moving that.

@prusnak
Copy link
Member

prusnak commented May 30, 2020

Please rework your PR. It now has a merge conflict after merging #89184

@rileyinman rileyinman force-pushed the rileyinman:iosevka-update branch from bd21c6d to 6d01e54 May 31, 2020
@rileyinman rileyinman changed the title iosevka, iosevka-bin: 2.3.3 -> 3.0.1 iosevka, iosevka-bin: 2.3.3 -> 3.1.0 May 31, 2020
@rileyinman
Copy link
Contributor Author

rileyinman commented May 31, 2020

@prusnak Rebased to master in order to fix. In the meantime a new update came out, so this no longer updates to 3.0.1 but rather 3.1.0.

@rileyinman rileyinman requested a review from asymmetric Jun 1, 2020
@asymmetric
Copy link
Contributor

asymmetric commented Jun 1, 2020

@rileyinman could you squash your commits following the guidelines?

@rileyinman rileyinman force-pushed the rileyinman:iosevka-update branch from bb6dcc0 to ed9225c Jun 2, 2020
@rileyinman rileyinman changed the title iosevka, iosevka-bin: 2.3.3 -> 3.1.0 iosevka, iosevka-bin: 2.3.3 -> 3.1.1 Jun 2, 2020
@rileyinman
Copy link
Contributor Author

rileyinman commented Jun 2, 2020

Commits squashed, in the meantime a bugfix release was made so the PR now updates to 3.1.1.

@rileyinman rileyinman force-pushed the rileyinman:iosevka-update branch from ed9225c to f367a53 Jun 4, 2020
@rileyinman
Copy link
Contributor Author

rileyinman commented Jun 4, 2020

Rebased to master to remove node conflict.

@asymmetric
Copy link
Contributor

asymmetric commented Jun 4, 2020

@rileyinman thanks for squashing :)

AFAIK though, each package should be its own commit. Here are the guidelines - they dont' mention this explicitly, so I guess it's up for interpretation, but that's how I would do it.

@rileyinman rileyinman force-pushed the rileyinman:iosevka-update branch from f367a53 to 3fab924 Jun 4, 2020
@rileyinman rileyinman changed the title iosevka, iosevka-bin: 2.3.3 -> 3.1.1 iosevka: 2.3.3 -> 3.1.1 Jun 4, 2020
@rileyinman
Copy link
Contributor Author

rileyinman commented Jun 11, 2020

Rebased to master to fix node-packages conflict.

Copy link
Contributor

asymmetric left a comment

I haven't tried this because it's trying to build something massive (node?), but LGTM.

Thanks for your work @rileyinman :)

@AluisioASG
Copy link
Contributor

AluisioASG commented Jun 21, 2020

Since this is still open, can you pass --jCmd=$NIX_BUILD_CORES after npm build -- so that it uses the right amount of cores?

@rileyinman rileyinman force-pushed the rileyinman:iosevka-update branch from 9e2b610 to 8a97e78 Jun 24, 2020
@rileyinman rileyinman changed the title iosevka, iosevka-bin: 2.3.3 -> 3.1.1 iosevka, iosevka-bin: 2.3.3 -> 3.2.2 Jun 24, 2020
@rileyinman
Copy link
Contributor Author

rileyinman commented Jun 24, 2020

@Alan01252 Done—I also updated it to the latest version and rebased to master to fix the merge conflict with node-packages.

Copy link
Contributor

asymmetric left a comment

The zsh_history file shouldn't have been created, I reckon.

@asymmetric
Copy link
Contributor

asymmetric commented Jun 24, 2020

@AluisioASG could you point me to the documentation of the --jCmd flag?

@rileyinman
Copy link
Contributor Author

rileyinman commented Jun 24, 2020

No idea how that got in there, good catch :P

@rileyinman rileyinman force-pushed the rileyinman:iosevka-update branch from 8a97e78 to 0015de0 Jun 24, 2020
@AluisioASG
Copy link
Contributor

AluisioASG commented Jun 24, 2020

@AluisioASG could you point me to the documentation of the --jCmd flag?

@asymmetric There isn't AFAIK, Iosevka's build system (verda) doesn't even have a source code link. I had to download the package from NPM and look around for a way to control the number of jobs (it defaults to the number of cores in the system and was overwhelming my VM).

@asymmetric
Copy link
Contributor

asymmetric commented Jul 5, 2020

@rileyinman could you rebase this please? It's a bit unfortunate this hasn't gotten a review from the maintainers yet - a friendly ping :)

@rileyinman
Copy link
Contributor Author

rileyinman commented Jul 5, 2020

@asymmetric Funny story, you just caught me right as I was doing that!

@rileyinman rileyinman force-pushed the rileyinman:iosevka-update branch from 0015de0 to 26d56ba Jul 5, 2020
@ttuegel
ttuegel approved these changes Jul 6, 2020
@nixos-discourse
Copy link

nixos-discourse commented Jul 7, 2020

This pull request has been mentioned on NixOS Discourse. There might be relevant details there:

https://discourse.nixos.org/t/prs-already-reviewed/2617/182

@ttuegel ttuegel merged commit daec48f into NixOS:master Jul 7, 2020
16 checks passed
16 checks passed
Evaluation Performance Report Evaluator Performance Report
Details
grahamcofborg-eval ^.^!
Details
grahamcofborg-eval-check-maintainers matching changed paths to changed attrs...
Details
grahamcofborg-eval-check-meta config.nix: checkMeta = true
Details
grahamcofborg-eval-darwin nix-instantiate --arg nixpkgs { outPath=./.; revCount=999999; shortRev="26d56ba"; rev="26d56baf0eaee71a01f9146ef9dc337b34a36c9a"; } ./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="26d56ba"; rev="26d56baf0eaee71a01f9146ef9dc337b34a36c9a"; } ./nixos/
Details
grahamcofborg-eval-nixos-manual nix-instantiate --arg nixpkgs { outPath=./.; revCount=999999; shortRev="26d56ba"; rev="26d56baf0eaee71a01f9146ef9dc337b34a36c9a"; } ./nixos/
Details
grahamcofborg-eval-nixos-options nix-instantiate --arg nixpkgs { outPath=./.; revCount=999999; shortRev="26d56ba"; rev="26d56baf0eaee71a01f9146ef9dc337b34a36c9a"; } ./nixos/
Details
grahamcofborg-eval-nixpkgs-manual nix-instantiate --arg nixpkgs { outPath=./.; revCount=999999; shortRev="26d56ba"; rev="26d56baf0eaee71a01f9146ef9dc337b34a36c9a"; } ./pkgs/t
Details
grahamcofborg-eval-nixpkgs-tarball nix-instantiate --arg nixpkgs { outPath=./.; revCount=999999; shortRev="26d56ba"; rev="26d56baf0eaee71a01f9146ef9dc337b34a36c9a"; } ./pkgs/t
Details
grahamcofborg-eval-nixpkgs-unstable-jobset nix-instantiate --arg nixpkgs { outPath=./.; revCount=999999; shortRev="26d56ba"; rev="26d56baf0eaee71a01f9146ef9dc337b34a36c9a"; } ./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
iosevka, iosevka-bin, iosevka-bin.passthru.tests, iosevka.passthru.tests on aarch64-linux Success
Details
iosevka, iosevka-bin, iosevka-bin.passthru.tests, iosevka.passthru.tests on x86_64-linux Success
Details
@rileyinman
Copy link
Contributor Author

rileyinman commented Jul 7, 2020

Thank you!

@rileyinman rileyinman deleted the rileyinman:iosevka-update branch Jul 7, 2020
@asymmetric
Copy link
Contributor

asymmetric commented Jul 13, 2020

Is it just me or are the variants (Etoile, Aile, ...) not being built?

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

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