Hello Liliana, thanks for your input on https://issues.guix.gnu.org/52575. I will continue the discussion about it here. You prepend most of your comments with "Why not do ...". I have an universal answer for that: I don't know :) I did what I did with the limited Guile knowledge at my disposal. I'm still learning everyday and therefore am thankful for your recommendations. > Is it really a good idea to use a string-encoded number here? Why not > a number? Why not two? A pair or list of symbols mayhaps? It should be a number, you are correct. > You maybe want to have a record here, but fair enough, a > string works too. I can see the benefit of that, but maybe that's something for another patch. I can add a `;;; MAYBE: ` comment to the implementation however if that's desired. > Why have a key network/on? Why not use 'on and 'network and document, > that they function the same due to (insert implementation detail). I think this point we need to discuss. I tried to implement things as close as possible to the "original" keys/values in https://git.kernel.org/pub/scm/bluetooth/bluez.git/tree/src/main.conf Specifically for the `privacy` key, you can see the implementation here https://git.kernel.org/pub/scm/bluetooth/bluez.git/tree/src/main.conf#n68, in particular --8<---------------cut here---------------start------------->8--- # Possible values for LE mode: "off", "network/on", "device" # Possible values for Dual mode: "off", "network/on", "device", # "limited-network", "limited-device" --8<---------------cut here---------------end--------------->8--- Now, I don't think it's a good idea to introduce arbitrary values for Guix, meaning split `network/on` in `'network` and `'on`. In my opinion (which may very well be wrong) the config values applicable in Guix should match those in the resulting config file as close as possible. For a user coming to Guix who already worked with the bluetooth configuration on another distro, seeing `'network` and/or `'on` (but not `'network/on`) as possible config values, is confusing, at least I think it is. > Why a list of UUIDs? Wouldn't it make more sense to use symbols like > 'debug, 'll-privacy, 'quality-report, etc.? > On that note, we have a UUID type, so use it. I actually didn't know there's a UUID type. This makes more sense. But not a list as only one value can be applied. Also I would advise introducing symbol like `'ll-privacy` and the like because of what I said earlier. > Should be a boolean. Same point here. I used boolean types whereever the config value need was `true`/`false`. In this particular case (I don't know why) the bluetooth implementations expects `0`/`1` instead of the former. This is why I kept it a number. However, in this case I think it's okay to deviate from the original implementation because `0` and false / `1` and `true` are universally understood as being equivalent (By this of course I mean the user would set `#f`/`#t`, the written value however would still be `0`/`1`). > Might want to rename/alias 'yes to 'paired. Again, same point here. `'paired` would be an arbitrary value only Guix have. > You probably want to write that as "an integer 7 <= N <= 16". ... >As above. Yes, absolutely. > Again, make sure that your code works with UUIDs here if it doesn't > already. Will do. > Use numbers, not strings. Will do. I'll create a new patch with all the things altered I agree about. I'll leave the rest until we have talked about the points I made. -- Best regards / Mit freundlichen Grüßen, Demis Balbach