-
Notifications
You must be signed in to change notification settings - Fork 15
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
Implement state management for remotes declared by nix-flatpak #32
Comments
Hey @ReedClanton , Thanks for the detailed issue report!
This is currently expected behavior. I appreciate remote management is a bit confusing (and not well explained). Thinking out loud: Assuming this is related to #31, where you started with a fresh NixOs install, I think you possibly hit a corner case where you ran module activation with a non-default remote declared at generation 0 (e.g. before running activation with only the default remote set). In this case,
Thinking out loud again.
I think that having As an alternative ways of managing
Both feel a bit clunky. |
That is what I did.
I don't really understand either of these options entirely. I don't know what And to be clear, when I say I don't entirely understand either option, I don't mean that I disagree with them. Rather I'm just new to Nix, so some stuff goes over my head.
Agreed. I can't think of an option that doesn't feel at least a little clunky. Maybe add a boolean option that when set to |
I was thinking about this while climbing, and I see three options that I like (although you may chose what every you want, obviously): Option 1DescriptionThis would be what you suggested (quoted bellow).
Example
Note: The above doesn't work, but I image you're suggestion would look something like it. Or maybe just explicitly include Option 2DescriptionMy currently preferred method would be to not have a default at all, as is done in the NixOS flatpak module I previously used. You've (@gmodena) expressed an opposition to this option, and I understand why. Example
Option 3DescriptionAdd an option to keep default remotes. Example
NoteRegardless of what change ends up being made, including no change, I strongly believe the docs should be updated to explicitly call out the behavior. |
Excellent sport activity 👍
There is an idiomatic
What makes this your preferred option? Honest question, I really appreciate feedback about UX. I keep going back and forth on this. On the one hand I like having a default (reduces boilerplate), on the other hand I think explicit is better than implicit. FWIW I'm currently leaning towards the status quo (with improved documentation).
Added some notes wrt how remotes config works, and how to manage the default value. |
I'm going to do some research tomorrow (and maybe the next day) so I can really make up my mind what I believe the best option is, however, I want to bring something up before I forget. I've mentioned before that the documentation should be updated at the least, but I also think that if a flatpak provided to |
ApproachBefore I started to answer this question, I wanted to determine what the most important factor(s) would be. Ultimately, I landed on consistency. In other words, the single most important factor when it comes to UX is consistency. It's not hard to image how application close buttons being in different locations in each application would make life difficult. I figured the same thing would apply here. It would be better to place all application close buttons in the same place, rather than move it into a spot that might work better for one application. ResearchI started by looking through Home Manager options. No option that accepted only a list had any default. The only exceptions I found were multi-type options (ex. programs.neomutt.binds.*.map programs.rofi.theme). Next I looked into how list type options are normally handled. Take AnalysisList type options, when provided by NixOS or Home Manager, universally have no default set. When a user sets a list type option, values are appended. Applying Personal ExperienceWhen I was in the process of switching over from declarative-flatpak to this module, I did the same thing I always do when adding something to my NixOS/Home Manager configuration. I looked at the options available, added each one I wanted to set, then ran my system build. My assumption that the default value would be appended to the values I added was the single largest sticking point. Counter Points
Response to Counter Points
ConclusionI believe this issue represents three distinct problems (listed in order of importance [according to me]):
I believe I did a poor job of writing this issue. It should be broken out into two issues. One that covers informing the use when flatpaks aren't installed, and another for addressing non-standard behavior/documentation. Next StepsA decision should be made regarding the path forward. @gmodena must make this diction. Regardless of what I think of it, I will accept it and help when/where I can. If @gmodena agrees, either @gmodena or @ReedClanton should write a separate issue for checking if flatpaks can be found in remotes and warning the user when they can't be. Note that this functionality will be internet dependent. So no errors/warnings should be produced when not connected to the internet. |
I just realized that the solution I purposed above will need to account for what remote will be used when not provided. Personally, I always provide both services.flatpak.remotes = [
{
default = true;
name = "flathub";
location = "someUrl";
}
{
name = "flathub-beta";
location = "someUrl";
}
]; alternate implementation: services.flatpak.remoteDefault = "flathub"; While I'd preffer the second implementation, if the first one is chosen, and multiple remotes are provided that have
Note: Also, choosing the second implementation would address the non-standard behavior of having a default remote because now |
Thanks for your analysis @ReedClanton . Super thoughtful and well put together. I will reply with appropriate depth as soon as I have some mental bandwidth to spare. |
Why would this be a bad idea? It could provide a lot of flexibility. User config defaults could look like:
Benefits:
So a minimum config to enable with added remote would be:
|
Hey @Lehmanator , thanks for your input. [...]
I think we achieve the same benefit you listed by piggybacking on In retrospect, I think it would have been better not to provide a default repo and let users manage their config explicitly. I might break this API in upcoming releases. |
I'm closing this issue since remote state management has been implemented. Might there be interested in API changes / follow ups, please open a thread in Discussions. |
Description
This repo's
README.md
states that Flathub is the default remote. It also explains how to add a remote. It does not explain that adding a remote will remove the default one.This is an issue because:
services.flatpak.remotes
being a list implies that any preexisting values will remain when new ones are added.Solution(s)
Add a note to the docs that explains the non-standard behavior. (easiest)
Convert
services.flatpak.remotes
from a list to a set. (this just seems like an all around bad option, but I'm including it anyways)Always append new values to remotes list (my preferred method). This could be implemented in a few ways:
The text was updated successfully, but these errors were encountered: