start-os icon indicating copy to clipboard operation
start-os copied to clipboard

Config: Do not render maps if all rows in the map are unrenderable (pointers, maps of pointers, maps of maps of pointers, etc.)

Open MattDHill opened this issue 3 years ago • 7 comments

Since there is nothing the user can do in this section, we should not be showing it. Can cause confusion and support load.

MattDHill avatar Apr 12 '22 01:04 MattDHill

The appropriate thing to do here is to not show any section if all of its subcomponents are pointers. we still want to group pointers because they get dereferenced to values in the config. Without this feature, it would force service packagers to put their pointers not into a common structure. This substantially harms Dev Experience.

Transferring to eos repo and renaming.

ProofOfKeags avatar Apr 14 '22 23:04 ProofOfKeags

After discussing with @MattDHill in person, we concluded that its best to change this in proxy for now as the solution to implement filtering of pointers from nested config elements might be non-trivial.

We want to be able to support the most streamlined dev experience, part of which means not enforcing strict bounds on how devs set up service configs. However, as it stands with how most projects are currently packaged, this would be an easy config change, with the only known service being affected is proxy.

elvece avatar Apr 25 '22 18:04 elvece

I'm rather opposed to reverting this improvement to Proxy. Doing so is undoing previous work in Proxy in order to avoid doing work that fundamentally needs to be done in the config rendering code. This is tacitly stating the requirement that objects whose rows consist solely of pointers are invalid.

Further this is not enough of a UX issue to justify regressing package code in order to punt the OS work. Consider this annoying config menu a persistent reminder that this feature still needs to be implemented. I recommend elevating this to P2

ProofOfKeags avatar Apr 28 '22 23:04 ProofOfKeags

I agree with @ProofOfKeags . The amount of work this will require to update proxy is not at all worth it.

chrisguida avatar Apr 29 '22 02:04 chrisguida

In the effort of not having scope creep for an unknown solution, I thought the change to proxy was a simple and happy medium that would eliminate current support load.

I also strongly feel that we should not be requiring devs to design their config in a particular way just to suit eOS.

Since the work to update the config rendering is unknown at this time, this issue should be moved to a current sprint as a spike and be completed in that sprint only if an easily known solution is determined. Otherwise outlining the solution for the next sprint is sufficient.

elvece avatar Apr 29 '22 18:04 elvece

Yeah this is not urgent. The status quo is not horrible.

MattDHill avatar Apr 29 '22 18:04 MattDHill

After discussing with @MattDHill in person, we concluded that its best to change this in proxy for now as the solution to implement filtering of pointers from nested config elements might be non-trivial.

We want to be able to support the most streamlined dev experience, part of which means not enforcing strict bounds on how devs set up service configs. However, as it stands with how most projects are currently packaged, this would be an easy config change, with the only known service being affected is proxy.

@elvece what is the "easy config change" you're referring to here? If I remove these items from the config, I have no access to bitcoind's rpc username and password. Am I missing something?

Edit: never mind, I'm dumb, you included it in your comment https://github.com/Start9Labs/btc-rpc-proxy-wrapper/issues/24

Yeah, I think filtering properly on the frontend is the correct solution here.

chrisguida avatar Jun 06 '22 20:06 chrisguida