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

fragment-mono: init at 1.21 #337557

Merged
merged 2 commits into from
Sep 30, 2024
Merged

fragment-mono: init at 1.21 #337557

merged 2 commits into from
Sep 30, 2024

Conversation

noahgitsham
Copy link
Contributor

Description of changes

Adds a new package for the Fragment Mono font.

It's already available in the google-fonts package, but nice to have it separated aswell (as seen in the roboto packages).

(This is my first nixpkgs PR, please let me know if I've made any mistakes)

Things done

  • Built on platform(s)
    • x86_64-linux
    • aarch64-linux
    • x86_64-darwin
    • aarch64-darwin
  • For non-Linux: Is sandboxing enabled in nix.conf? (See Nix manual)
    • sandbox = relaxed
    • sandbox = true
  • Tested, as applicable:
  • Tested compilation of all packages that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage
  • Tested basic functionality of all binary files (usually in ./result/bin/)
  • 24.11 Release Notes (or backporting 23.11 and 24.05 Release notes)
    • (Package updates) Added a release notes entry if the change is major or breaking
    • (Module updates) Added a release notes entry if the change is significant
    • (Module addition) Added a release notes entry if adding a new NixOS module
  • Fits CONTRIBUTING.md.

Add a 👍 reaction to pull requests you find important.

pkgs/top-level/all-packages.nix Outdated Show resolved Hide resolved
pkgs/data/fonts/fragment-mono/default.nix Outdated Show resolved Hide resolved
pkgs/data/fonts/fragment-mono/default.nix Outdated Show resolved Hide resolved
@noahgitsham
Copy link
Contributor Author

@AndersonTorres Ah apologies forgot to rtfm on pkgs specifically, let me fix all this.

@noahgitsham noahgitsham force-pushed the fragment-mono branch 3 times, most recently from 535bc1c to 7c53153 Compare August 26, 2024 22:13
pkgs/by-name/fr/fragment-mono/package.nix Outdated Show resolved Hide resolved
pkgs/by-name/fr/fragment-mono/package.nix Outdated Show resolved Hide resolved
pkgs/by-name/fr/fragment-mono/package.nix Show resolved Hide resolved
pkgs/by-name/fr/fragment-mono/package.nix Outdated Show resolved Hide resolved
@noahgitsham
Copy link
Contributor Author

Is there some OfBorg command someone/me has to run to get it to run/skip those two Darwin tests or is there just a long unskippable queue? This package has no passthru.tests which is what it says it's waiting to run.

@ryand56
Copy link
Member

ryand56 commented Aug 27, 2024

Is there some OfBorg command someone/me has to run to get it to run/skip those two Darwin tests or is there just a long unskippable queue? This package has no passthru.tests which is what it says it's waiting to run.

It's a long unskippable queue due to having less Darwin machines and the fact that Darwin is slow and not able to be emulated that well.

@AndersonTorres
Copy link
Member

Squash the commits according to contributing guidelines.

@noahgitsham
Copy link
Contributor Author

Squash the commits according to contributing guidelines.

What's wrong with the commits? Are the last two not covered by the etc in (pkg-name): (from -> to | init at version | refactor | etc)

@AndersonTorres
Copy link
Member

AndersonTorres commented Sep 2, 2024

A newly-added package does not need a commit for each fixup:

https://github.com/NixOS/nixpkgs/blob/master/CONTRIBUTING.md#commit-conventions

If you have commits pkg-name: oh, forgot to insert whitespace: squash commits in this case.
Use git rebase -i. See Squashing Commits for additional information.

Fixup the three last commits into one.

@noahgitsham
Copy link
Contributor Author

A newly-added package does not need a commit for each fixup:

I see, could be clearer to write that explicitly in CONTRIBUTING.md, as I assumed oh, forgot to insert whitespace only meant formatting changes.

@AndersonTorres
Copy link
Member

I see, could be clearer to write that explicitly in CONTRIBUTING.md, as I assumed oh, forgot to insert whitespace only meant formatting changes.

The idea behind this is more general, and a bit hard to express in words.
The idea is that small fixups - including but not limited to formatting - do not need an entry in the commit history.

Further, there is another unwritten rule: the first time a package is included in Nixpkgs, it should be included as if it was done perfectly.
Therefore, even big fixups like "add changelog link" or "add Python as dependency (thanks EggSpam for noticing this!)" should not appear in the commit history of a new package.

(Obviously there are exceptions, such as including a package that requires a modification in another existing package - but this is another story...)

@noahgitsham
Copy link
Contributor Author

Don't mean to sound impatient, there's no rush, but I'm curious as to why this hasn't been merged yet.

Does each PR need multiple manual approvals? Or can I run the merge bot command to do it now?

@AndersonTorres
Copy link
Member

Don't mean to sound impatient, there's no rush, but I'm curious as to why this hasn't been merged yet.

Because no human with the power to push the button pushed it yet.

Does each PR need multiple manual approvals?

No. It just needs a human with the power to push the button to push it.

Or can I run the merge bot command to do it now?

The merge bot will not push the button in this case.
I will not elaborate, but the merge bot merges only PRs that came from the auto-updater bot, and only in specific circumstances.


Summarizing, you need to find a human with the power to push the button.

There are some places you can find them:

@nixos-discourse
Copy link

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

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

@noahgitsham
Copy link
Contributor Author

Great, thanks for all the help! This feels like a very welcoming community.

@h7x4 h7x4 changed the title Add new font package: Fragment Mono fragment-mono: init at 1.21 Sep 30, 2024
@h7x4 h7x4 merged commit de0d993 into NixOS:master Sep 30, 2024
31 checks passed
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.

7 participants