-
-
Notifications
You must be signed in to change notification settings - Fork 1.9k
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
devilspie2: add module #1477
devilspie2: add module #1477
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM.
If you could add a simple test like the one from sxhkd it would be great 👍
Thank you for your contribution! I marked this pull request as stale due to inactivity. If this remains inactive for another 7 days, I will close this PR. Please read the relevant sections below before commenting. If you are the original author of the PR
If you are not the original author of the issue
|
Reopening. Looks like this was pretty close to done. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just a couple of nits. Sorry for the delay getting back to you on this!
devilspie2-configuration = ./configuration.nix; | ||
devilspie2-service = ./service.nix; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I feel like these tests can be combined into one, but I think it would be fine to leave separate as well.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree, these tests should be combined.
Co-authored-by: Sumner Evans <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Everything looks good 😄.
Would you mind merging the tests together?
While these tests should be independent, the nature of the test utility makes it that running a test and evaluating it is non-trivial, increasing the time it takes to evaluate the entire test suite.
devilspie2-configuration = ./configuration.nix; | ||
devilspie2-service = ./service.nix; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree, these tests should be combined.
According to the devilspie2 website, it looks like the current maintainer is looking for a new maintainer, and the last commit was two years ago. Furthermore Gnome has defaulted to Wayland for a while now, and I believe this only works on X. I'm not sure how long Gnome plans to support X11, but the future of this utility is not looking good at the moment? So you could probably close this if @dawidsowa doesn't happen to come back to it. |
Co-authored-by: Nicolas Berbiche <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, LGTM!
Description
Adds Devilspie2 as a service.
It's useful for window managers with limited options for windows rules, such as mutter (gnome).
Checklist
Change is backwards compatible.
Code formatted with
./format
.Code tested through
nix-shell --pure tests -A run.all
.Test cases updated/added. See example.
Commit messages are formatted like
See CONTRIBUTING for more information and recent commit messages for examples.
If this PR adds a new module
Added myself as module maintainer. See example.
Added myself and the module files to
.github/CODEOWNERS
.