Skip to content

nixos/prosody: add muc_notifications module support #434574

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
Open
kuznetsss wants to merge 1 commit into
base: master
Choose a base branch
from
Open

nixos/prosody: add muc_notifications module support #434574

kuznetsss wants to merge 1 commit into from

Conversation

@kuznetsss
Copy link
Contributor

@kuznetsss kuznetsss commented Aug 17, 2025 •
edited
Loading

muc_notifications is a community module recommended to be enabled by Monal client. As I understand it should be used in a different way comparing to the other community modules by adding it into modules_enabled of muc module.

I added logic to use muc_notifications community module in the correct way and added test to check generated configuration.

Things done

  • Built on platform:
    • x86_64-linux
    • aarch64-linux
    • x86_64-darwin
    • aarch64-darwin
  • Tested, as applicable:
  • Ran nixpkgs-review on this PR. See nixpkgs-review usage.
  • Tested basic functionality of all binary files, usually in ./result/bin/.
  • Nixpkgs Release Notes
    • Package update: when the change is major or breaking.
  • NixOS Release Notes
    • Module addition: when adding a new NixOS module.
    • Module update: when the change is significant.
  • Fits CONTRIBUTING.md, pkgs/README.md, maintainers/README.md and other READMEs.

Add a 👍 reaction to pull requests you find important.

@nixpkgs-ci nixpkgs-ci bot added 10.rebuild-linux: 1-10 This PR causes between 1 and 10 packages to rebuild on Linux. 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin. 6.topic: nixos Issues or PRs affecting NixOS modules, or package usability issues specific to NixOS 8.has: module (update) This PR changes an existing module in `nixos/` labels Aug 17, 2025
@kuznetsss kuznetsss force-pushed the Add_muc_notiffications_support_for_prosody branch from 59938bd to 0d4285c Compare August 17, 2025 23:02
@toastal
Copy link
Contributor

toastal commented Aug 18, 2025 •
edited
Loading

Not entirely sure on the value of that NixOS test. Spinning up a VM to test that Nix’s strings interpolation works seems pretty wasteful. If a test were/needed to be written, shouldn’t it test that notification are actually happening with a client or something similar for this to provide value?

@kuznetsss kuznetsss force-pushed the Add_muc_notiffications_support_for_prosody branch from 0d4285c to 16a2b74 Compare August 18, 2025 19:56
@kuznetsss
Copy link
Contributor Author

kuznetsss commented Aug 18, 2025

@toastal, I removed my simple test and I don't know how to create such an advanced test you described. I think if generated config matches module usage from documentation then it should be fine.
Sorry, I don't really know how to check my changes. If you could share a link or explain how to do it, I will test it manually.

@nixpkgs-ci nixpkgs-ci bot added the 2.status: merge conflict This PR has merge conflicts with the target branch label Aug 21, 2025
@kuznetsss kuznetsss force-pushed the Add_muc_notiffications_support_for_prosody branch from 16a2b74 to 907616c Compare August 31, 2025 17:44
@nixpkgs-ci nixpkgs-ci bot removed the 2.status: merge conflict This PR has merge conflicts with the target branch label Aug 31, 2025
@kuznetsss kuznetsss force-pushed the Add_muc_notiffications_support_for_prosody branch from 907616c to 0c1c835 Compare August 31, 2025 17:53
@kuznetsss
Copy link
Contributor Author

kuznetsss commented Aug 31, 2025

@toastal, I manually checked that prosody.lua is generated as expected with and without muc_notifications module. Please review this PR again when you have time.

SuperSandro2000
SuperSandro2000 reviewed Aug 31, 2025
View reviewed changes
@kuznetsss kuznetsss force-pushed the Add_muc_notiffications_support_for_prosody branch from 0c1c835 to 0a5789c Compare August 31, 2025 21:40
@nixos-discourse
Copy link

nixos-discourse commented Oct 8, 2025

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

https://discourse.nixos.org/t/prs-ready-for-review/3032/5928

@kuznetsss
Copy link
Contributor Author

kuznetsss commented Oct 16, 2025

@toastal, could you please review when you have a moment?

toastal
toastal reviewed Oct 17, 2025
View reviewed changes
@kuznetsss kuznetsss force-pushed the Add_muc_notiffications_support_for_prosody branch 3 times, most recently from 215ad8a to 4c30c3b Compare October 21, 2025 22:27
@kuznetsss
Copy link
Contributor Author

kuznetsss commented Nov 11, 2025

@SuperSandro2000, could you please review?

@kuznetsss
Copy link
Contributor Author

kuznetsss commented Nov 30, 2025

@toastal, sorry for making noise, but maybe you could review/approve when you have time?

toastal
toastal reviewed Dec 1, 2025
View reviewed changes
toastal
toastal reviewed Dec 1, 2025
View reviewed changes
@kuznetsss kuznetsss force-pushed the Add_muc_notiffications_support_for_prosody branch from 4c30c3b to adb6759 Compare December 1, 2025 22:31
@kuznetsss kuznetsss changed the title prosody: add muc_notifications module support nixos/prosody: add muc_notifications module support Dec 1, 2025
@kuznetsss kuznetsss force-pushed the Add_muc_notiffications_support_for_prosody branch from adb6759 to 7282c22 Compare December 1, 2025 22:36
toastal
toastal reviewed Dec 2, 2025
View reviewed changes
@kuznetsss kuznetsss force-pushed the Add_muc_notiffications_support_for_prosody branch 2 times, most recently from 962b51c to 4089e02 Compare December 2, 2025 23:04
@kuznetsss kuznetsss force-pushed the Add_muc_notiffications_support_for_prosody branch from 4089e02 to 3036333 Compare December 2, 2025 23:22
@nixpkgs-ci nixpkgs-ci bot added the 12.approvals: 1 This PR was reviewed and approved by one person. label Dec 3, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@SuperSandro2000 SuperSandro2000 Awaiting requested review from SuperSandro2000

1 more reviewer

@toastal toastal toastal approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

6.topic: nixos Issues or PRs affecting NixOS modules, or package usability issues specific to NixOS 8.has: module (update) This PR changes an existing module in `nixos/` 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin. 10.rebuild-linux: 1-10 This PR causes between 1 and 10 packages to rebuild on Linux. 12.approvals: 1 This PR was reviewed and approved by one person.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@kuznetsss @toastal @nixos-discourse @SuperSandro2000