Hi, Denis 'GNUtoo' Carikli: >> The phone number lookup service leaks private information so hide the >> option to enable it. > It may be a good idea to give a bit more context by explaining that it > needs to be hidden because the "Disable phone number lookup settings > during upgrade" only disable the settings when upgrading the database.
this patch is not really related to the upgrade patch that disables these settings. If one has a installation of latest replicant dev + this patch on their device then the lookup feature a) is always disabled b) cannot be enabled because the settings are hidden. If it makes it any more clear I can put it for v2 like this: Hide information leaking phone number lookup settings The phone number lookup service leaks private information so hide the option to enable it. The lookup service is by default disabled so this guarantees no information gets leaked if the user would accidentally enable the service. >> LookupSettingsFragment.class.getName(); >> - target.add(lookupSettingsHeader); >> + // target.add(lookupSettingsHeader); > Here it would be better to instead remove completely the line, or if you > want to leave something, add a text comment instead that explains what > was removed and why. > > Note that I didn't have time to look into the broader context of that > patch yet, so I don't know if lookupSettingsHeader should be completely > removed or not. I will remove it completely in v2. Thanks for the review! Joonas _______________________________________________ Replicant mailing list [email protected] https://lists.osuosl.org/mailman/listinfo/replicant
