mozilla :: #screenshots

11 Jul 2017
00:02GitHub[screenshots] 6a68 force-pushed localize-server from 16b1050 to 59b6f10:
00:02GitHubscreenshots/localize-server c235178 Jared Hirsch: l10n - nearly there
00:02GitHubscreenshots/localize-server 59b6f10 Jared Hirsch: Still poking at the last step
01:31GitHub[screenshots] mozilla-pontoon pushed 1 new commit to master:
01:31GitHubscreenshots/master 5633a30 paul.trevor: Pontoon: Update German (de) localization of Firefox Screenshots...
01:42circleci-botFailed: mozilla-pontoon's build (#1969; push) in mozilla-services/screenshots (master) --
09:34GitHub[screenshots] SoftVision-PaulOiegas reopened issue #3056: [Enhancement] Add an icon next to the save button to make it more self-evident
10:14GitHub[screenshots] niharikak101 force-pushed fxa from 2afc0cd to b750905:
10:14GitHubscreenshots/fxa b750905 Niharika Khanna: let user modify shot info
10:26circleci-botFailed: niharikak101's build (#1970; push) in mozilla-services/screenshots (fxa) --
15:47niharikaI lost connectivity for the last 5 seconds. Apologies
15:50_6a68fastest OKR review ever!
16:00clouserwsorry. I shouldn't try to squeeze it into a stand-up meeting
16:00clouserwjust trying to avoid scheduling more meetings for everyone
16:00ianbickingwe made some progress ;)
16:00_6a68I totally understand, I kinda felt like we covered enough ground to iterate on them
16:01_6a68ianbicking: would you mind taking a look at the l10n PR? suspect I'm missing something simple
16:07_6a68ianbicking: hmm, maybe the issue is that the LocalizationProvider is outside the body tag?
16:07_6a68er, outside the head tag. the body tag wraps the BodyTemplate, so that's no answer
16:07ianbicking_6a68: I hadnt yet actually checked out the code, but thats possible the head and body are handled slightly differently
16:08_6a68what's a nice workflow for debugging react components that aren't doing their job?
16:09ianbickingoh, I just realized our chronic test failures are keeping the dev server from updating
16:09ianbickingbut there I go getting distracted with things
16:12_6a68oh, I remember why I put the head tag inside the LocalizationProvider: I was hitting a React.Children.only error, and wasn't sure of the source
16:12ianbickingI dont really understand LocalizationProvider, but it seems more-or-less legit
16:13ianbicking_6a68: do you know how it passes things to children? Not through props apparently
16:13_6a68so, the docs say that you pass it a generator function that returns MessageContext objects, then I forget how it passes those to the children, one sec
16:14_6a68ianbicking: it uses context,
16:15_6a68note that the docs expect a generator function; the last little commit in that branch was a wild swing at "what if I just pass in an array" and might need to be reverted
16:16ianbicking_6a68: it says iterable, so an array should be fine
16:16* ianbicking appreciates JS borrowed all the Python terminology in this case
16:17_6a68heh, yeah
16:17ianbickingwe should move staticLink and a few other things to context too
16:17ianbickingbut only if it works, of course
16:18ianbicking_6a68: so its not working even for English right now?
16:19jgruenianbicking: i'm just going to email ritu directly to get her sense of whether those context fill patches are good candidates for uplift
16:19_6a68ianbicking: right, the variable substitutions are working for the nested Localized elements, but if you edit the placeholder text that's inside the html, it doesn't get replaced with the strings
16:19ianbickingjgruen: cool. You can tell her we can make the Screenshots export today with or without it, depending on that other ticket
16:19_6a68I am baffled that the substitutions work - for instance, the bold words in the headings on the homepage - but the actual strings aren't swapped in there
16:20ianbicking_6a68: Im getting an error "$number is not defined - is that the kind of thing you are thinking of?
16:21ianbickingsince the page renders, but is broken in the browser, I wonder if thats some part of it?
16:21_6a68what page are you getting that on? I thought I had fixed those errors, I was trying to inline replacements, and it wasn't working
16:21ianbicking_6a68: on a shot page
16:22ianbicking_6a68: on load. bullet is not defined if I try to interact with the expiration widget
16:22* _6a68 tries
16:23ianbicking_6a68: also, if you look at the page source: window.initialModel = {"authenticated":true,"sentryPublicDSN":"","backend":"http://localhost:10080","gitRevision":"8.1.0-185-g59b6f10","csrfToken":"QhaJGTDy-JNFEYB5lJd2sVB6zG2Kbd5YHbMU","abTests":{},"messages":[], ...
16:23ianbicking_6a68: the messages list is empty
16:23_6a68oh, now that's a good clue
16:23_6a68that's funny, because when I log messages inside reactrender, it's nonempty
16:24ianbicking_6a68: well, not empty on the server
16:24_6a68messages isn't a JSONable list of strings, it's a MessageContext, one of the fluent-react components
16:24ianbickingthat might explain the variables too, doesnt have a variable $number
16:24_6a68that explains it, I think
16:24ianbickingArray.from(messages) then?
16:25_6a68yeah, the annoying thing is that LocalizationProvider expects an iterable of MessageContexts, and I had wrapped the context creation in l10n.js
16:25_6a68tied the pretzel slightly backwards
16:26_6a68thanks for pointing that out, ian. I can just pass the ftl messages from l10n.js, and assemble the MessageContext in the browser code
16:51jgruenianbicking: emailed ritu, will keep you posted
17:50GitHub[screenshots] mozilla-pontoon pushed 1 new commit to master:
17:50GitHubscreenshots/master c5904fc paul.trevor: Pontoon: Update German (de) localization of Firefox Screenshots...
18:59circleci-botFailed: 6a68's build (#1972; push) in mozilla-services/screenshots (master) --
19:40jgruenianbicking: hey, so i think we should go ahead and cut 10.4 with the context-fill patch in it
19:40jgruenthen we should tag the two context-fill bugs as blockers
19:41ianbickingjgruen: theres two? Can you link them in ?
19:44GitHub[screenshots] ianb commented on issue #3099: Export will depend on and maybe one other bug @johngruen will identify
19:44firebotBug 1379464 NEW, Enable context paint for moz-extension:// images
19:45GitHub[screenshots] ianb commented on issue #3109: Landing details have been figured out
19:45GitHub[screenshots] ianb closed pull request #3109: add context fill icons (master...use-context-fill)
19:45GitHub[screenshots] ianb deleted use-context-fill at c1c114d:
19:50GitHub[screenshots] johngruen commented on issue #3099: The two issues blocking this are as follows:...
19:53GitHub[screenshots] ianb pushed 2 new commits to v10.4.0-export:
19:53GitHubscreenshots/v10.4.0-export 7cb237f John Gruen: add context fill icons
19:53GitHubscreenshots/v10.4.0-export 1bcbdda Ian Bicking: Change version to 10.4.0 with changelog
19:53GitHub[screenshots] ianb tagged 10.4.0 at v10.4.0-export:
19:55circleci-botFailed: ianb's build (#1974; push) in mozilla-services/screenshots (master) --
19:57GitHub[screenshots] ianb pushed 1 new commit to master:
19:57GitHubscreenshots/master e2a5575 Ian Bicking: Add note about creating bug
19:58GitHub[screenshots] ianb commented on issue #3099: Export bug: [Bug 1380120](
19:58firebotBug 1380120 NEW, Update Screenshots to version 10.4.0
19:59GitHub[screenshots] johngruen commented on issue #3099: @ianb
19:59firebotBug 1380119 NEW, Update Screenshots add-on to 10.4
19:59jgruenianbicking: let's use mine for more context :)
19:59jgruenbesides i beat you to it!
20:00ianbickingjgruen: youre breaking our process!
20:01jgruenack, okay, want me to close mine and reopen yours with my notes?
20:02ianbickingjgruen: oh wait, you dupd mine? Too many cooks!
20:02ianbickingjgruen: I can update yours, thats fine
20:02jgruenyeah, sorry
20:02jgrueni didn't realize you were doing that
20:03jgrueni can close mine if you like and re-open yours
20:03ianbickingno, its fine
20:03GitHub[screenshots] msujaws commented on issue #3109: Why did the filenames have to change to add "-v2"? We should have been able to keep the old filenames, right?
20:03ianbickingjgruen: ^ for you!
20:04GitHub[screenshots] johngruen commented on issue #3109: @msujaws yes most likely, but i was encoutering some caching issues with the old icons while testing so i changed the names in order to pick up the diff.
20:05GitHub[screenshots] msujaws commented on issue #3109: probably needed to run `./mach build` instead of `./mach build faster` to fix the caching issue. i've seen that before with changes to the pocket system addon
20:06GitHub[screenshots] ianb commented on issue #3109: Our normal development process sideloads the add-on via `web-ext`, so @johngruen doesn't typically use mach.
20:07GitHub[screenshots] johngruen commented on issue #3109: @msujaws jaws, maybe so. would you prefer i revert the names or leave as is?
20:07circleci-botFailed: ianb's build (#1976; push) in mozilla-services/screenshots (master) --
20:07GitHub[screenshots] msujaws commented on issue #3109: well, it's your project so do as you wish, but building version numbers in to filenames is doing a job that our version control system already does :)
20:09GitHub[screenshots] johngruen commented on issue #3109: got it. let's keep it as since it's make-work to revert it now. The -v2 will stand as a memorial of what's happened here :)
20:11jgruenianbicking: ritu recommended we request uplift on everything tomorrow for beta 9 which gets tagged thursday
20:24ianbickingjgruen: the bug is all set
20:30jgruenianbicking: thansk
20:31jgruensorry i messed up ur flow
20:31ianbickingjgruen: n/p ;) though releases stress me out, so Im more focused on habits
21:22GitHub[screenshots] ianb created fix-tests (+1 new commit):
21:22GitHubscreenshots/fix-tests ba5a815 Ian Bicking: Fix #3039, update tests for full-page preview...
21:22GitHub[screenshots] ianb opened pull request #3110: Fix #3039, update tests for full-page preview (master...fix-tests)
21:32circleci-botFailed: ianb's build (#1978; push) in mozilla-services/screenshots (fix-tests) --
21:34relud-ptoianbicking: cc clouserw: I've been informed that Screenshots instances are trying to create their s3 bucket on startup or something. can you verify that, and perhaps allow it to be turned off. if the app isn't doing it then we might have reason to think something is suspicious.
21:35ianbickingrelud-pto: yes, were doing that
21:35GitHub[screenshots] ianb opened issue #3111: Make S3 bucket creation configurable
21:36GitHub[screenshots] ianb pushed 1 new commit to fix-tests:
21:36GitHubscreenshots/fix-tests 6349557 Ian Bicking: Remove trailing whitespace
21:38clouserwstartup meaning, the first time a screenshot is taken? (or first time the button is clicked, I guess?)
21:39clouserwif we turned that off, what would that mean for storing screenshots?
21:45circleci-botFixed: ianb's build (#1979; push) in mozilla-services/screenshots (fix-tests) --
22:19GitHub[screenshots] 6a68 commented on issue #3110: bumped the main commit to see if it'll pass on a second try
22:25_6a68jgruen: heads up, photon onboarding will include an intro to screenshots:
22:25firebotBug 1366041 ASSIGNED, Add "Take Screenshot" button to Page Action Menu
22:25_6a68in case you need to coordinate with the photon UXers
22:25jgruenafaik it's just using UITour to highlight the button
22:25jgruenfor now anyway
22:28circleci-botFailed: ianb's build (#1980; retry by 6a68) in mozilla-services/screenshots (fix-tests) --
22:47GitHub[screenshots] 6a68 commented on issue #3110: ah, failed on a trailing whitespace linter error. let's land it
22:47GitHub[screenshots] 6a68 pushed 1 new commit to master:
22:47GitHubscreenshots/master 3b610cb Ian Bicking: Fix #3039, update tests for full-page preview (#3110)...
22:47GitHub[screenshots] 6a68 closed issue #3039: Figure out why CircleCI tests are failing
22:47GitHub[screenshots] 6a68 deleted fix-tests at 6349557:
23:01circleci-botFixed: 6a68's build (#1981; push) in mozilla-services/screenshots (master) --
23:01cloudops-ansiblescreenshots-dev build #149: building mozilla/screenshots:latest
23:04cloudops-ansiblescreenshots-dev build #149: mozilla/screenshots:latest deployed to Dev
12 Jul 2017
No messages
Last message: 74 days and 20 hours ago