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

discord: combine nixcord, vencord, vesktop #854

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

Flameopathic
Copy link
Contributor

@Flameopathic Flameopathic commented Feb 10, 2025

combines the nixcord, vencord, and vesktop hm.nix modules into one.

handles the config separately as the nixcord module requires odd conditions for evaluation.

unsure how to handle the testbeds.

@Flameopathic Flameopathic marked this pull request as draft February 10, 2025 20:56
@Flameopathic Flameopathic force-pushed the discord-module branch 2 times, most recently from c8ce86d to f0ad74f Compare February 10, 2025 20:59
@trueNAHO
Copy link
Collaborator

trueNAHO commented Feb 10, 2025

unsure how to handle the testbeds.

@danth, /modules/firefox/ suffers from the same problem of having one testbed per module, although multiple testbeds would be beneficial.

What do you think of optional /modules/<MODULE>/testbeds/<TESTBED>.nix testbeds, while asserting that /modules/<MODULE>/testbed.nix should not exist while any /modules/<MODULE>/testbeds/<TESTBED>.nix exists. However, this prevents the use of a highly convenient local /modules/<MODULE>/testbeds/common.nix abstraction. Either, we handle certain file names differently (probably a bad idea) or such files are expected to go to /modules/<MODULE>/common.nix. Since we are looking for specific files and directories inside /modules/<MODULE>/, adding arbitrary named files in this scope does not cause problems.

Note that implementing this functionality should probably not fall in the scope of this PR. It should be fine to keep only one testbed in this PR and add the others back once this functionality is implemented.

@Flameopathic Flameopathic force-pushed the discord-module branch 2 times, most recently from f950658 to e6d3fd8 Compare February 11, 2025 02:28
@Flameopathic Flameopathic marked this pull request as ready for review February 11, 2025 02:33
@danth
Copy link
Owner

danth commented Feb 11, 2025

@trueNAHO That sounds reasonable. Ignoring specific file names would be confusing and limiting, so I agree, it's better to just import from the parent directory when some shared code is needed.

modules/discord/hm.nix Outdated Show resolved Hide resolved
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants