mozilla :: #activity-stream

11 Sep 2017
13:44streaminatorMardak: Hey, your patch was approved! https://github.com/mozilla/activity-stream/pull/3400
14:27ursulak88hudson: https://github.com/mozilla/activity-stream/pull/3395 is good to review now :D
14:29streaminatorahillier: Hey, your patch was approved! https://github.com/mozilla/activity-stream/pull/3387
14:29streaminatorahillier: Hey, your patch was approved! https://github.com/mozilla/activity-stream/pull/3387
14:36mconleyk88hudson: heyo - I was curious (for no reason in particular), how's the pre-rendering stuff going? Did you find solutions to all of the state storage / customization problems we talked about a few weeks back?
14:37streaminatorcsadilek: Hey, your reviewer requested some changes: https://github.com/mozilla/activity-stream/pull/3361
14:37streaminatorcsadilek: Hey, your reviewer requested some changes: https://github.com/mozilla/activity-stream/pull/3361
14:55streaminatorursula: Hey, your patch was approved! https://github.com/mozilla/activity-stream/pull/3395
14:55streaminatorursula: Hey, your patch was approved! https://github.com/mozilla/activity-stream/pull/3395
15:04streaminatorMardak: Hey, your patch was approved! https://github.com/mozilla/activity-stream/pull/3403
16:16k88hudsonmconley: we decided to go with a simpler version for now (basically we just point to the prerendered version if certain pref conditions are true in aboutNewTabService) but we've filed a ticket to do the state storage stuff post-57
16:17mconleyk88hudson: wicked
16:21k88hudsonmconley: yeah like that stuff you and asuth were talking about seems promising, just will require a little more work
17:29k88hudsonMardak: ok I fixed the test / exported the aboutRedirector stuff in https://bugzilla.mozilla.org/show_bug.cgi?id=1398819, what's the bug it should depend on?
17:30Mardakprobably https://bugzilla.mozilla.org/show_bug.cgi?id=1398239
17:32flodMardak: I've received this question twice already but don't know the answer. How frequently is the system add-on updated?
17:33flodI'm looking at https://hg.mozilla.org/mozilla-central/log/tip/browser/extensions/activity-stream/data/locales.json, but frequency is all over the place
17:33flod(French guy is asking, since he made a typo)
17:33Mardakflod: we try to export at least once a week but leading up to 57 beta merge we might do it 2 or 3 times?
17:33flodok
17:33flodthanks (and reported back)
17:33Mardakwe just merged on friday and we will probably do one today
17:35k88hudsonMardak: perfec tthanks
18:05jchaulkI'll be a few min late joining triage... last meeting running over
18:53dmoseandreio: https://github.com/mozilla/activity-stream/blob/master/docs/v2-system-addon/telemetry.md will likely be useful for the bad state bug
18:54dmoseandreio: the dashboard user story now gets filed mozilla/ping-centre ; i need to update the checklist
18:55andreiodmose: can you edit the bug with the signals we are interested in for this bug? It's not obvious to me, I don't have enough context about telemetry.
18:56dmoseandreio: they're in the comment 0
18:56dmosethere are two
18:58andreiok88hudson: https://arxiv.org/abs/1709.02755 this has no metadata for me
18:58andreiodmose: so it
18:59andreiotop_sites_data_ready_ts and highlight_data_ready_ts
18:59dmoseyeah
18:59andreiook thanks.
19:00dmosesure thing
19:00dmosethank you!
19:04andreioMardak: I'm not sure what you mean by "places query" when you mentioned ProfileAge vs places query. Do you mean querying for the oldest bookmark to determine cutoff timestamp?
19:05Mardakandreio: yeah. both are async. one does DB queries. the other.. might read a pref ?
19:09Mardakandreio: looks like it does it 3 ways. both choices involve file IO, but i suppose using ProfileAge is more just calling an API than creating our own custom query. let's go with ProfileAge
19:09Mardakhttps://searchfox.org/mozilla-central/source/toolkit/modules/ProfileAge.jsm#31-42
19:10Mardakoh hey it just reads a times.json file from the profile directory :p
19:11Mardakk88hudson: just making sure, we can't land the aboutNewTabService change without first exporting?
19:11Mardakit would try to load a non-existent -prerendered.html file
19:11k88hudsonMardak: yeah, because it will start pointing at a non-existent file
19:11k88hudsonMardak: if it's easier I can change it to set .prerender to false
19:11k88hudsonwhat would you prefer
19:12k88hudson(like by default)
19:18andreioMardak: I would go with ProfileAge. The other method seems like a workaround
19:18Mardakk88hudson: it looks like with pref false, it will skip the need to fix up all_files_referenced for -prerendered.html
19:19k88hudsonah darn
19:19Mardakk88hudson: that's a good thing. we don't need to patch the test then unpatch
19:19k88hudsonoh right, we already wrote the patch
19:19k88hudsonlol
19:19k88hudsonMardak: let's just land it after then
19:20Mardakk88hudson: well either way. if we land service change first, it needs to be pref false then pref true later without touching all_files_ref. if we export first, we need to patch all_files_ref but then only need to pref prerender=true once
19:22Mardakk88hudson/andreio: this needs to be fixed before we can export though https://bugzilla.mozilla.org/show_bug.cgi?id=1398739#c4
19:22Mardak(that's what prevented me from exporting prerendered on friday)
19:22k88hudsonah, darn
19:23k88hudsonMardak: ok i'll take a look at that
19:23k88hudsonor andreio are you working on that?
19:27Mardakso.. an initial look leaves me confused. doing a "foo" search with google from context menu, original about:home, etc results in https://www.google.com/search?q=foo&ie=utf-8&oe=utf-8&client=firefox-b
19:27Mardakwhich seems to be what activity stream does too....
19:27Mardaklooks like the test isn't looking for the "&client=firefox-b" part ?
19:27Mardak"https://www.google.com/search?q=foo&ie=utf-8&oe=utf-8&client=firefox-b" == "https://www.google.com/search?q=foo&ie=utf-8&oe=utf-8"
19:31Mardakoh oh oh hooh o https://bugzilla.mozilla.org/show_bug.cgi?id=1315953
19:31Mardakdmose even commented on that bug :p
19:31Mardakmaybe that failure doesn't actually exist
19:32Mardakohhh dohhhhh skip-if = artifact # bug 1315953
19:37k88hudsonMardak: I can't seem to reproduce it
19:38Mardakk88hudson: artifact / non?
19:38k88hudsonartifact
19:38k88hudsonah
19:38Mardakwe might be good to export with andreio's existing patch in mozreview
19:39Mardakdoing some more sanity checks
20:01andreiosorry all its 11pm here im bad at following chat at this hour
20:01andreiodont count on me to reply
20:10Mardaknanj: do you see this failure? FAIL | browser/base/content/test/general/browser_favicon_change_not_in_document.js | Favicon image should correspond to expected image.
20:11k88hudsonWho wants to review https://github.com/mozilla/activity-stream/pull/3416
20:11nanjMardak: yes, i have fixed it. making a new patch for it
20:11k88hudsonthat's the render english string
20:22streaminatork88hudson: Hey, your patch was approved! https://github.com/mozilla/activity-stream/pull/3416
20:24Mardakk88hudson: we should be good to export? i'll update strings and you can make the patch for https://bugzilla.mozilla.org/show_bug.cgi?id=1398239 ?
20:28Mardakk88hudson: should be https://github.com/mozilla/activity-stream/compare/3dbf720c906457fec53af01877ebd2408ce5d863...96d0c785936b68b434a5903cbfe9fdc44e9e62ff
20:28k88hudsonMardak: can do
20:28Mardakit'll need both m-c-patches files applied as part of the export
20:28k88hudsonMardak: what do you mean update the strings
20:28ursulaMardak: if we wait like 30m i'll have a patch with small icons that i'd like to get in on the export
20:29k88hudsonursula: i'm cool with that
20:30ursulacool i'm just writing a test now
20:32Mardakk88hudson: was doing a strings-import from -l10n
20:33Mardakno more french double interesting? ;) Retrouvez des pages inintressantes
20:34k88hudsonLOOOOL
20:35k88hudsonthat's like retrieve uninteresting pages
20:35k88hudsonhaha ok
20:35Mardakoh. my french is rusty. in- does negate :p
20:36k88hudsonoh wait i could be wrong
20:36Mardakgoogle says it's uninteresting ;) https://translate.google.com/#auto/en/inint%C3%A9ressantes
20:36k88hudsongood catch whoever found it
20:39streaminatork88hudson: Hey! Someone requested a review: https://github.com/mozilla/activity-stream/pull/3417
20:54Mardakahillier: https://github.com/mozilla/activity-stream/issues/3410#issuecomment-328655404
20:54Mardakmight be able to fix 2 issues at once ;) .. maybe...
20:58ahillierMardak: That sounds good to me :)
20:59streaminatorursula: Hey, your patch was approved! https://github.com/mozilla/activity-stream/pull/3417
20:59streaminatorursula: Hey, your patch was approved! https://github.com/mozilla/activity-stream/pull/3417
21:01dmoseMardak and/or K88Hudson: got a minute to chat re RemotePageManager asynchrony fun?
21:01Mardakdmose: sure
21:01Mardakdmose: in your vidyo
21:02dmoseok
21:03ursulaok export is good to go :) thanks k88hudson for the quick review
21:03k88hudsonursula: np!
21:03k88hudsondmose: sure
21:12k88hudsondmose: Mardak: are yall good with the remotepagemanager stuff or should i join
21:12Mardakk88hudson: we're okay
21:12k88hudsonok cool
21:12Mardakwell.. trying to figure it out :p
21:13k88hudsonMardak: are you done with the strings
21:13Mardakk88hudson: yeah
21:14k88hudsoncool
21:31Mardakk88hudson: was there some webpack change that resulted in stuff like const {locale, strings, initialized} = props.App; not being expanded?
21:31k88hudsonMardak: where are you seeing that?
21:32k88hudsonthere shouldn't be
21:32Mardakk88hudson: cant seem to link directly. https://reviewboard.mozilla.org/r/178514/diff/1#index_header search for "_ref2"
21:32k88hudsonah, I see it
21:32k88hudsonweird
21:32Mardakit's fine. (better) but was curious
21:37Mardakk88hudson: did you mean to include a resource://activity-stream/img/newtab-icon.svg ?
21:37Mardak(there isn't one)
21:39k88hudsonMardak: looks like that path is wrong
21:40Mardakwhere did that come from? i believe we should be just using the brand icon from a few lines up?
21:40k88hudsonI have no idea
21:40k88hudsonyeah
21:40k88hudsoni'm trying to figure out where it's from :P
21:41Mardakk88hudson: https://github.com/mozilla/activity-stream/pull/3389/files#diff-35258b1218232a26af6060648ea59692L21
21:41k88hudsonaaaaaah
21:41k88hudsonLOL
21:42Mardakk88hudson: also for the title. just leave empty?
21:44k88hudsonMardak: cool
21:46Mardakk88hudson: !!!!!!!!!!! the webpack destructuring change makes it so we can do let [, foo, bar] = [1,2,3]; without it injecting unused code that instabul complains about !
21:47k88hudsonMardak: :o
21:47k88hudsoni wonder how that happened
21:47k88hudsonahhhhhhh
21:47k88hudsoni bet i know what it is
21:48k88hudsonwhen ahillier removed a bunch of our old dependencies, it might have changed the requirements on some subdependencies
21:48k88hudsonwe could look at the package.lock diff
21:48streaminatork88hudson: Hey, your patch was approved! https://github.com/mozilla/activity-stream/pull/3418
22:16Mardakdmose: took less time to include url than trying to figure out what we can use from the browser :p
22:17dmoseMardak: heh, good times. thanks!
22:24k88hudsonMardak: uh oh, i'm seeing lots of test failures
22:25k88hudsonMardak: oh wait nevermind
22:25k88hudsonwrong try tab
22:25Mardakdmose: and look at that. r+ already ;) https://bugzilla.mozilla.org/show_bug.cgi?id=1398955
22:28Mardakk88hudson: well it just went through on autoland so.... we'll see! :p
22:29k88hudsonMardak: ha fair enough
12 Sep 2017
No messages
   
Last message: 10 days and 12 hours ago