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

Automtaically add ipv6 entries to nix flake #2803

Merged
merged 3 commits into from
Jan 23, 2025

Conversation

Pandapip1
Copy link
Contributor

Solves #47 for NixOS

Copy link

welcome bot commented Jan 17, 2025

Thank you for submitting this pull request! We’ll get back to you as soon as we can!

@Pandapip1
Copy link
Contributor Author

Just tested it on my NixOS system, this works.

@Pandapip1 Pandapip1 marked this pull request as ready for review January 17, 2025 19:43
@Pandapip1 Pandapip1 force-pushed the ipv6-flake branch 3 times, most recently from 39d72a5 to cf34678 Compare January 17, 2025 19:49
@StevenBlack
Copy link
Owner

Thank you for this Gavin @Pandapip1.

Can someone please validate this?

Ping Dennis @Deleh or @toastal how does this PR appear to you?

@toastal
Copy link
Contributor

toastal commented Jan 20, 2025 via email

@Deleh
Copy link
Contributor

Deleh commented Jan 20, 2025

I agree with @toastal. The code looks good and functionality is as expected, but it would make sense to keep the additional entries behind a mkEnableOption. E.g. blockAAAARequests or blockIPv6.

The outcome of networking.stevenBlackHosts.enable should reflect the hosts file as it is.

@Pandapip1
Copy link
Contributor Author

Pandapip1 commented Jan 20, 2025

Also went ahead and fixed your mkEnableOption usage.

Before:

nix-repl> (nixosModule { config = { networking.enableIPv6 = true; }; }).options.networking.stevenBlackHosts.blockSocial.description
"Whether to enable Additionally block social hosts.."

After:

nix-repl> (nixosModule { config = { networking.enableIPv6 = true; }; }).options.networking.stevenBlackHosts.blockSocial.description
"Whether to enable social hosts entries."

@Deleh
Copy link
Contributor

Deleh commented Jan 21, 2025

A default value of config.networking.enableIPv6 seems fine for me. The changes were tested and work as expected 👍

Can you please extend the README accordingly?

@toastal
Copy link
Contributor

toastal commented Jan 21, 2025 via email

@StevenBlack
Copy link
Owner

Thank you for the assist Dennis @Deleh and @toastal. You folks are great!

Gavin @Pandapip1 this looks good to merge except for one subtle thing.

The root readme.md file is generative. There are actually 31 different "homepage" readme.md files in this repository, one for each distinct variant of hosts file. These are auto-generated from the file readme_template.md.

THEREFORE if you could please apply the readme changes to the file readme_template.md then this'll be good to merge.

@StevenBlack
Copy link
Owner

StevenBlack commented Jan 23, 2025

Thanks Gavin @Pandapip1! Merging.

@StevenBlack StevenBlack merged commit 9f4ceb2 into StevenBlack:master Jan 23, 2025
12 checks passed
Copy link

welcome bot commented Jan 23, 2025

Congratulations on merging your first pull request here! 🎉🎉🎉 You are now in our list of contributors. Welcome!

@Pandapip1 Pandapip1 deleted the ipv6-flake branch January 23, 2025 04:15
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.

4 participants