Skip to content

refactor(extractors/services): simplify logic#136

Merged
oddlama merged 2 commits intooddlama:mainfrom
KP64:refactor/services
May 3, 2026
Merged

refactor(extractors/services): simplify logic#136
oddlama merged 2 commits intooddlama:mainfrom
KP64:refactor/services

Conversation

@KP64
Copy link
Copy Markdown
Contributor

@KP64 KP64 commented Mar 28, 2026

Checks are simplified or removed entirely if unnecessary. Usage of nicer functions. Should be non-breaking.

Checks are simplified or removed entirely if unnecessary. Usage of nicer functions. Should be non-breaking.
@oddlama
Copy link
Copy Markdown
Owner

oddlama commented Apr 15, 2026

I think there are some nice ones in there, but a lot of or null and or false statements got removed which break backward compat with older nixpkgs releases

@KP64
Copy link
Copy Markdown
Contributor Author

KP64 commented Apr 15, 2026

I think there are some nice ones in there, but a lot of or null and or false statements got removed which break backward compat with older nixpkgs releases

No I don't think so unless I made a mistake on the way. In fact was this whole redundant null checking and stuff are the primary motivation for this PR. If you see the option definitions of the NixOS modules they don't allow null values rendering the checks redundant.
Let's take Forgejo for instance:

          info = mkIf (
            config.services.forgejo.settings ? server.ROOT_URL
          ) config.services.forgejo.settings.server.ROOT_URL;
          details.listen = mkIf (address != null && port != null) { text = "${address}:${toString port}"; };

It checks whether there is an address and a port. If we look at the current stable release (25.11) then we can see here that they are always defined and guaranteed to be non null.

Edit: Seems like I can't read. "older nixpkgs releases" is plural referring to even older (e.g. 24.11, etc.). @oddlama what nixpkgs releases should we as maintainers keep in mind? Imo we should at some point remove the checks to keep everything tidy. I don't see a benefit to support "ancient" nixpkgs releases. Do you have a policy for that in mind?

@oddlama
Copy link
Copy Markdown
Owner

oddlama commented Apr 15, 2026

Imo we should at some point remove the checks to keep everything tidy. I don't see a benefit to support "ancient" nixpkgs releases. Do you have a policy for that in mind?

Currently we just have no official statement on what we support. I guess it would be fair to say we support the current unstable and the latest stable release only, in which case you are right and the checks can be removed.

I'll have to read through this thoroughly tomorrow before merging. If we introduce more or null in the future, we also might want to mark them for removal on next stable branchoff with a FIXME marker or similar

@oddlama
Copy link
Copy Markdown
Owner

oddlama commented May 3, 2026

Let's do this and commit to only latest stable + unstable support
Thanks!

@oddlama oddlama merged commit 28e9dc9 into oddlama:main May 3, 2026
1 check passed
@KP64 KP64 deleted the refactor/services branch May 3, 2026 11:43
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.

2 participants