-
Notifications
You must be signed in to change notification settings - Fork 322
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
gluon-config-mode-contact-info: an "obligatory" option in site.conf #843
gluon-config-mode-contact-info: an "obligatory" option in site.conf #843
Conversation
First Second, I think the fact that the owner information is public is a reason why you do not want to have this as "obligatory" but let the people decide on their own. |
So what exactly do I have to change in the makefile to get this accepted? is it this part here? https://github.com/freifunk-gluon/gluon/pull/843/files#diff-83df6372edb9e411399cb8c89df9d143R32 second: obligatory in this package only means, that you have to fill in anything (even just a space is ok if you want to stay anonymous) |
nice @rubo77 , we're planning on doing something similar, having a "are you sure you want to leave the contact field empty? ..." message appear, so the people installing nodes at least don't forget it by accident. @kb-light well, one could argue the PPA specifices you have to be reachable as a node operator.. |
@rotanid, good idea. Did you create a packet for this already? I added the standalone packet, we already tested in the first message. We assume, that at the moment many users keep the contact field empty, cause of laziness or they want to fill it out later, but never find the time. only a small fraction doesn't want to be reachable. I also thought of an option like: [x] I would like to be notified, if my node is offline
I think, if a popup asks for confirmation to leave the field empty is not strong enough. If the field is obligatory, we get a better hint in the |
@@ -0,0 +1,35 @@ | |||
local cbi = require "luci.cbi" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this whole file is in the wrong folder, a while ago it switched places, maybe you started your development based on an older revision of gluon.
@rubo77 Such a checkbox doesn't really make sense, because to spam an address you have to have confirmed opt-in by that person, meaning that you also verified the opt-in really came from the owner of the address. So if you have a monitoring solution, simply let it send an "You're node went down for the first time, do you want to receive such notices in the future?". (I think you're using my nodewatcher? That still has to be implemented there.) |
570c2e8
to
5d8712f
Compare
I moved anything else? |
.. 'this information will be visible <em>publicly</em> ' | ||
.. 'on the internet together with your node\'s coordinates.' | ||
) | ||
) | ||
|
||
local o = s:option(cbi.Value, "_contact", i18n.translate("Contact info")) | ||
o.default = uci:get_first("gluon-node-info", "owner", "contact", "") | ||
o.rmempty = true | ||
o.rmempty = not site.owner.obligatory |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Have you tested this in case owner
is not set within site.conf
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes.
it worked as false (or nil), anyway, it worked fine. I guess lua does automatic typecasting in case it is nil
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if obligatory
is not set everything works fine, but in case owner
is not set, there will be an error: attempt to index field 'owner' (a nil value)
So you should change this to: o.rmempty = not (site.owner or {}).obligatory
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ok, since I refactored the option to site.config_mode.owner.obligatory
I changed this to
o.rmempty = not ((site.config_mode or {}).owner or {}).obligatory
6113920
to
cfb4196
Compare
@@ -181,13 +181,18 @@ | |||
|
|||
-- Skip setup mode (config mode) on first boot | |||
-- setup_mode = { | |||
-- skip = true, | |||
-- skip = true, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
there really is a space missing, but that has nothing to do with this pull request
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
should I revert this correction then? or just leave it, as minor, uninteresting side-fix ;-)
cfb4196
to
edd116f
Compare
@@ -13,10 +13,15 @@ PKG_CONFIG_DEPENDS += $(GLUON_I18N_CONFIG) | |||
define Package/gluon-config-mode-contact-info | |||
SECTION:=gluon | |||
CATEGORY:=Gluon | |||
TITLE:=Set a custom string that will be distributed in the mesh. | |||
TITLE:=Set a custom string for a contact info that will be distributed in the mesh. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"for a contact info" sounds weird, "information" is always plural. Also the whole sentence doesn't make a lot of sense.
Maybe "Allows the user to provide contact information to be distributed in the mesh"
dab2b6a
to
5578de9
Compare
Ich hab den Satz noch mal überarbeitet:
|
Das klingt jetzt seltsam, als könnte man auch einen nicht öffentlichen Hinweis hinterlegen... Vielleicht das erste "öffentlich" streichen und statt dessen das zweite (oder sogar "öffentlich im Internet") hervorheben? |
|
5578de9
to
0a93ad1
Compare
DEPENDS:=gluon-config-mode-core-virtual +gluon-node-info | ||
endef | ||
|
||
define Package/gluon-config-mode-contact-info/description | ||
Set a custom string for a contact info that will be distributed in the mesh. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You updated the TITLE, but left the "a contact info" in the description.
OK, so? "Bitte hinterlege hier einen Hinweis, um anderen " |
0688d57
to
d3c03b7
Compare
d3c03b7
to
4172125
Compare
Ich würde hier auch gerne Chat-Nickname ergänzen:
Aber dies besser in einem extra PR oder? |
We already have a useable stand-alone packet to replace the
gluon-config-mode-contact-info
package in our repository at:https://github.com/freifunk-kiel/ffki-packages/tree/master/gluon-config-mode-contact-obligatory