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.

Sign up for GitHub

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

Jump to bottom

i3: 4.23 -> 4.24 #355441

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

i3: 4.23 -> 4.24 #355441

wants to merge 5 commits into from
+106 −35

Conversation

fpletz
Copy link
Member

@fpletz fpletz commented Nov 12, 2024

https://i3wm.org/downloads/RELEASE-NOTES-4.24.txt

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.

JohnRTitor
JohnRTitor reviewed Nov 12, 2024
View reviewed changes
Copy link
Contributor

@JohnRTitor JohnRTitor left a comment

Choose a reason for hiding this comment

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

If you are refactoring anyway, perhaps it's time to move to finalAttrs.

JohnRTitor
JohnRTitor reviewed Nov 12, 2024
View reviewed changes
Copy link
Contributor

@JohnRTitor JohnRTitor left a comment

Choose a reason for hiding this comment

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

https://github.com/NixOS/nixpkgs/pull/355441/files#diff-e5b9d43660301a4bc15bc160023f39f2f9d2fd275bab589350d38f05b61f6cadR106

lib.getExe here

@fpletz
Copy link
Member Author

fpletz commented Nov 12, 2024

If you are refactoring anyway, perhaps it's time to move to finalAttrs.

Yup, I'll modernize the whole file. Thanks for the hint.

JohnRTitor
JohnRTitor reviewed Nov 12, 2024
View reviewed changes
Copy link
Contributor

@JohnRTitor JohnRTitor left a comment

Choose a reason for hiding this comment

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

runHook preCheck
and postCheck

https://github.com/NixOS/nixpkgs/pull/355441/files#diff-e5b9d43660301a4bc15bc160023f39f2f9d2fd275bab589350d38f05b61f6cadL53

drupol
drupol approved these changes Nov 12, 2024
View reviewed changes
Copy link
Contributor

@drupol drupol left a comment

Choose a reason for hiding this comment

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

LGTM

pkgs/applications/window-managers/i3/default.nix Outdated Show resolved Hide resolved
@ofborg ofborg bot added 11.by: package-maintainer This PR was created by the maintainer of the package it changes 10.rebuild-darwin: 11-100 10.rebuild-linux: 11-100 labels Nov 13, 2024
JohnRTitor
JohnRTitor approved these changes Nov 13, 2024
View reviewed changes
Copy link
Contributor

@JohnRTitor JohnRTitor left a comment

Choose a reason for hiding this comment

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

Wanted to merge but

@ofborg why are you so slow

@drupol
Copy link
Contributor

drupol commented Nov 13, 2024

Wanted to merge but

@ofborg why are you so slow

I don't know either, it's been like that since a week or so :(

@wegank wegank added the 12.approvals: 2 This PR was reviewed and approved by two reputable people label Nov 13, 2024
@JohnRTitor
Copy link
Contributor

@grahamc or @NixOS/ofborg-maintainers I wonder what's up with it. Some machines down? Been noticing this for a week as well, previously it will take maximum three-four hours to eval, now it's more than twelve.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@JohnRTitor JohnRTitor JohnRTitor approved these changes

@drupol drupol drupol approved these changes

Assignees
No one assigned
Labels
10.rebuild-darwin: 11-100 10.rebuild-linux: 11-100 11.by: package-maintainer This PR was created by the maintainer of the package it changes 12.approvals: 2 This PR was reviewed and approved by two reputable people
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@fpletz @drupol @JohnRTitor @wegank