Feature #14833
Point to the survey from /home
0%
Description
Subtasks
History
#1 Updated by sajolida 2017-10-11 17:56:45
- Assignee changed from sajolida to segfault
- QA Check set to Ready for QA
- Feature Branch set to web/14833-veracrypt-survey
segfault: Do you mind reviewing this? It’s a tiny bit of HTML+CSS+JavaScript.
But otherwise I’ll find someone else…
#2 Updated by segfault 2017-10-11 19:50:45
Why is this commit on this branch?
commit d6f428ad41672d29b0b7fc9d970cf8874bb95aa3
Author: sajolida <sajolida@pimienta.org>
Date: Wed Oct 11 15:13:52 2017 +0000
Add past timeline of DAVE 2
#3 Updated by sajolida 2017-10-12 13:45:00
Because I forgot to push that same commit in origin/master. Done now and the Git log of origin/master..origin/web/14833-veracrypt-survey is now clean.
#4 Updated by segfault 2017-10-13 12:55:39
- Assignee changed from segfault to sajolida
I think your probability calculation is incorrect:
var n = 20; // Probability of displaying the survey button is 1/n
function displaySurvey() {
if(Math.floor(n * Math.random()) == 0) {I think to get a probability of 1/n it should be
if(Math.floor(n/2 * Math.random()) == 0)Besides that it looks fine. But I was not able to build the website to look at the result, because the build currently fails on my system (with an error which I also get on master, so this has nothing to do with your branch).
Can we say that this passed the review without me building the website? Or should someone else do this?
#5 Updated by sajolida 2017-10-14 19:27:34
- Assignee changed from sajolida to segfault
I fixed a problem in the build with 4b9992cd70, so maybe it was that.
Regarding the calculation, I’m not sure. On the one head I believe you’re much better at maths and algorithmic than, but on the other hand I tested my calculation and I think it does what I want. I want to link to the survey to appear on 1 out of each 20 pages, randomly. That’s 1/n, with n=20, right?
With my calculations if I do console.log(Math.floor(n * Math.random())); 20 times I get: 17, 8, 14, 4, 8, 14, 16, 19, 16, 17, 0, 3, 19, 9, 11, 2, 7, 0, 14, 10 and 0 is one out of 20 possibilities.
With your calculation if I do console.log(Math.floor(n/2 * Math.random())); 20 times I get: 9, 1, 5, 1, 3, 1, 7, 2, 9, 2, 1, 9, 1, 1, 9, 1, 6, 6, 0, 3 and 0 is one out of 10 possibilities.
What am I missing? :/
#6 Updated by sajolida 2017-10-17 21:33:39
Tonight I replaced the underline on “Contribute” with a button to have a stronger call for action.
I really wanted to have the survey life, so I pushed it even if we’re not settled on the probability calculation. But that’s ok, and we can fit it later on.
So I’m still interested in your review but please take your time!
#7 Updated by segfault 2017-10-17 22:01:08
Oops, I overlooked the email about your comment #5, else I would have replied earlier.
> I fixed a problem in the build with 4b9992cd70, so maybe it was that.
No, my build still fails with
building news.fa.po
Use of uninitialized value $file in concatenation (.) or string at /usr/share/perl5/IkiWiki.pm line 963.> I want to link to the survey to appear on 1 out of each 20 pages, randomly. That’s 1/n, with n=20, right?
This is also what I understood from your comment in the code. And you are right, your code works, I was thinking about round() instead of floor(). Sorry!
#8 Updated by sajolida 2017-10-18 17:26:28
- Status changed from Confirmed to Resolved
- Assignee deleted (
segfault)
The build works on my machine and on Jenkins so it’s on your side. Happy to debug your ikiwiki setup once we meet (but not remotely!)
Ack on floor instead of round. I feel better about my maths now! :)
#9 Updated by intrigeri 2018-12-02 21:45:26
- QA Check deleted (
Ready for QA)