Skip to content
This repository has been archived by the owner on Mar 28, 2019. It is now read-only.

Fix Bug 1213020 - Update nightly.mozilla.org to include B2GDroid builds #46

Merged
merged 1 commit into from Nov 11, 2015
Merged

Fix Bug 1213020 - Update nightly.mozilla.org to include B2GDroid builds #46

merged 1 commit into from Nov 11, 2015

Conversation

kyoshino
Copy link
Contributor

No description provided.


<!-- B2GDroid promo -->
<!-- Spotlight Activity Tag: Mozilla (6247) | Mozilla B2G Activity Tag 2 (52162) | Mozilla Spots (4669) | Expected URL: -->
<script>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We already have a JS file for code; JS should go in https://github.com/mozilla/nocturnal/blob/master/js/nightly.js

@tofumatt
Copy link
Contributor

r-

Sorry, but I think this is the wrong approach to this problem. To add an item to the nightly site it needs to be added to scrape.py so it needs to be accessible from the Mozilla FTP server.

Let me know if you need any guidance.

@kyoshino
Copy link
Contributor Author

The campaign code was provided in Bug 1213905 and permanent link was provided in Bug 1213020. Since the file is not on FTP, and because of the campaign code, I just hardcoded the link.

@tofumatt
Copy link
Contributor

The tracking code seems very weird to me; we already have Google Analytics and wonder why we don't use that.

Adding the HTML directly is harder to maintain and also puts your new build at the very top of the page, which I don't think is best (desktop browsers should be at the top of the nightly page).

The nightly site isn't set up to handle static URLs right now, so that functionality needs to be added into the Python script. Would you be interested in doing this?

@kyoshino
Copy link
Contributor Author

I'm working on it.

@kyoshino
Copy link
Contributor Author

Okay, now it looks much better. The hardcorded HTML is gone, JS is in the separate file. I don't know why we don't use GA events here neither, though.

@tofumatt
Copy link
Contributor

tofumatt commented Nov 11, 2015 via email

@kyoshino
Copy link
Contributor Author

Thanks @tofumatt!

ftGoalTagPix52162.src = "http://servedby.flashtalking.com/spot/8/6247;52162;4669/?spotName=Mozilla_B2G_Activity_Tag_2&cachebuster="+num;

event.preventDefault();
return false;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You shouldn't need return false; as you're already using event.preventDefault();

@tofumatt
Copy link
Contributor

See proposed code tweaks, but after that this is good, thanks!

Ping me when they're made and I'll merge it in and we can file the push request with IT.

@m-alexis
Copy link

Kohei, can you swap out the tag with 52280?

@kyoshino
Copy link
Contributor Author

Fixed the suggested points except the arrangement, and swapped the campaign tag.

@tofumatt
Copy link
Contributor

tofumatt commented Nov 11, 2015 via email

@pmclanahan
Copy link
Contributor

FWIW I agree with @tofumatt that Desktop should be the prominent product on the site.

@m-alexis
Copy link

jbertsch is fine with this going below desktop.

@kyoshino I don't think this was called out explicitly, but we should make sure the implementation of this tracking pixel respects users with DNT.

@kyoshino
Copy link
Contributor Author

Okay, will update the code shortly.

@kyoshino
Copy link
Contributor Author

Added a DNT check and rearranged the order to put B2GDroid between Mobile (renamed the label to Android for clarity) and Desktop B2G like this:

screen shot 2015-11-11 at 15 31 47

tofumatt added a commit that referenced this pull request Nov 11, 2015
Fix Bug 1213020 - Update nightly.mozilla.org to include B2GDroid builds
@tofumatt tofumatt merged commit d64f838 into mozilla:master Nov 11, 2015
@kyoshino kyoshino deleted the bug-1213020-b2gdroid branch November 11, 2015 23:56
@tofumatt
Copy link
Contributor

Thanks so much for all the work on this patch; sorry it took so long.

Filed the deploy bug: https://bugzilla.mozilla.org/show_bug.cgi?id=1224035

@kyoshino
Copy link
Contributor Author

Thanks @tofumatt for your timely review!

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants