rustdesk / rustdesk-server

RustDesk Server Program
https://rustdesk.com/server
GNU Affero General Public License v3.0
5.96k stars 1.22k forks source link

change variable name for clarity #379

Closed tschettervictor closed 4 months ago

tschettervictor commented 4 months ago

changed "rustdesk_hbbs_ip" to "rustdesk_hbbr_server" as that is what the variable is referring to according to the docs

tschettervictor commented 4 months ago

After messaging with the the maintainer of the port, I'm not 100% sure how much this PR will affect existing users.

The maintainer @madpilot78 believes it would be too much hassle to change the variable, as that might force existing users to adjust their setup, while I believe the variable should be changed for the sake of having things easy to understand and implement. If a variable is not specific as to what it is referring to, that caused some confusion. It did for me anyway, that is why I'm pushing this PR.

That said, I will leave the decision up to @rustdesk to decide.

madpilot78 commented 4 months ago

I think some context is missing.

What @tschettervictor means with "the port" is, the FreeBSD port existing in the FreeBSD Ports Collection, specifically this one: https://www.freshports.org/net/rustdesk-server/

I maintain that port and took the rc scripts from the rustdesk-server distribution, adapting them just slightly for the package. I left the names of the variables unmodified.

Since this package is installed by users and they have configured their systems with these variables, my issue with changing the variables is, as described, that users will suddenly need to modify their systems configuration when upgrading the package. Some users will end up with broken installation because they are not aware or forget to change this configuration, and this is something we try to avoid in the FreeBSD ports (and base system too).

This could also affect the users of other packages on other OSes.

I also think the variable name is not wrong as is. Variables names are no substitute for documentation. I improved the variable description in the rc script in the port here, I could file a PR for a similar change.