[00:02:14] <grrrit-wm>	 (03Merged) 10jenkins-bot: Fix another event handler bug [extensions/MultimediaViewer] (wmf/1.23wmf7) - 10https://gerrit.wikimedia.org/r/101144 (owner: 10MarkTraceur)
[00:28:51] <grrrit-wm>	 (03CR) 10Aarcos: "Ping !" [extensions/MultimediaViewer] - 10https://gerrit.wikimedia.org/r/100733 (owner: 10Aarcos)
[01:13:54] <marktraceur>	 Eloquence: FYI, fix merged and backported, should be good everywhere
[01:14:06] <marktraceur>	 Yay lightning deploys.
[17:43:15] <grrrit-wm>	 (03PS5) 10Aarcos: Add smoke test to class mw.LightboxInterface. - Moved tests that were meant for LightboxInterface objects. - Fixed space naming issues. [extensions/MultimediaViewer] - 10https://gerrit.wikimedia.org/r/100733 
[17:58:27] <fabriceflorin>	 marktraceur tgr aarcos Hey guys! I'm working from home today, so I will join via Google Hangouts. See you there in a couple minutes.
[17:58:37] <marktraceur>	 Ahhh, 'kay.
[18:25:38] <grrrit-wm>	 (03CR) 10MarkTraceur: [C: 04-1] "Itty bitty issues" (033 comments) [extensions/MultimediaViewer] - 10https://gerrit.wikimedia.org/r/100733 (owner: 10Aarcos)
[18:39:27] <grrrit-wm>	 (03PS6) 10Aarcos: Add smoke test to class mw.LightboxInterface. - Moved tests that were meant for LightboxInterface objects. - Fixed space naming issues. [extensions/MultimediaViewer] - 10https://gerrit.wikimedia.org/r/100733 
[18:39:54] <grrrit-wm>	 (03CR) 10Aarcos: "PTAL" [extensions/MultimediaViewer] - 10https://gerrit.wikimedia.org/r/100733 (owner: 10Aarcos)
[18:41:54] <grrrit-wm>	 (03CR) 10MarkTraceur: [C: 032] "And proper dependency injection and wonderful tests and yay. /merge" (031 comment) [extensions/MultimediaViewer] - 10https://gerrit.wikimedia.org/r/100733 (owner: 10Aarcos)
[18:43:03] <grrrit-wm>	 (03Merged) 10jenkins-bot: Add smoke test to class mw.LightboxInterface. - Moved tests that were meant for LightboxInterface objects. - Fixed space naming issues. [extensions/MultimediaViewer] - 10https://gerrit.wikimedia.org/r/100733 (owner: 10Aarcos)
[18:48:12] <bd808>	 dan-nl: Aaron just gave +2 on the fix to core that your temp file issue inspired: https://gerrit.wikimedia.org/r/#/c/101132/
[19:30:38] <dan-nl>	 hey bd808 did that merge go to beta?
[19:31:02] <grrrit-wm>	 (03PS4) 10Gergő Tisza: Rewrite template parsing with DOMDocument [extensions/CommonsMetadata] - 10https://gerrit.wikimedia.org/r/100569 
[19:31:08] <bd808>	 dan-nl: I imagine it should be there by now
[19:31:16] <dan-nl>	 gwtoolset can no longer create a temp file ...
[19:31:25] <bd808>	 o_O
[19:31:38] <dan-nl>	 2013-12-13 18:55:55 deployment-jobrunner08 commonswiki: gwtoolsetUploadMediafileJob User:Dan-nl/GWToolset/Mediafile_Batch_Job/52ab583932055 options=array(3) whitelisted-post=array(39) user-name=Dan-nl user-options=array(20) t=1570 error=GWToolset\Jobs\UploadMediafileJob::run: Could not create temporary file.
[19:32:00] <dan-nl>	 i then tried a new upload batch and it runs a preview of 3 items first
[19:32:10] <dan-nl>	 that gave a similar message ...
[19:33:29] <grrrit-wm>	 (03CR) 10Gergő Tisza: "Thanks! Fixed most issues, renamed DomCrawler to DomNavigator." (033 comments) [extensions/CommonsMetadata] - 10https://gerrit.wikimedia.org/r/100569 (owner: 10Gergő Tisza)
[19:33:30] * dan-nl  trying it out locally

[19:36:02] <bd808>	 dan-nl: Do you know which operation throws that exception?
[19:36:16] <dan-nl>	 trying to narrow it down now
[19:38:01] <dan-nl>	 the key is tmp-create-error
[19:38:53] <dan-nl>	 looks like it's in reallyFetchFile
[19:42:39] <dan-nl>	 bd808 hmm .. maybe it's a perm issue on beta? works for me locally
[19:44:19] <bd808>	 permissions on deployment-jobrunner08:/tmp look good, but maybe that's not where it's trying to write now?
[19:44:40] * bd808  searches some files

[19:44:57] <dan-nl>	 that could be … will update my local repo so i have the latest mw
[19:47:06] <bd808>	 dan-nl: The core patch is bad
[19:47:12] <bd808>	 I'll try to fix it
[19:47:43] <dan-nl>	 k
[19:52:02] <dan-nl>	 bd808: would this take care of it ?
[19:52:03] <dan-nl>	 $tmpFile = new TempFSFile( tempnam( wfTempDir(), 'URL' ) );
[19:52:35] <bd808>	 dan-nl: https://gerrit.wikimedia.org/r/#/c/101256/
[19:53:14] <bd808>	 There is a factory method that I apparently thought was the constructor when I wrote the original patch
[19:53:17] <bd808>	 :/
[19:53:35] <dan-nl>	 ah
[19:53:56] <dan-nl>	 returns /var/tmp//URL7376a5bc3c3c-1 … the older was returning /private/var/tmp/URLS7JIU3
[19:54:06] <dan-nl>	 guess that doesn't matter ...
[19:54:35] <dan-nl>	 i'll verify it for you ...
[19:54:38] <bd808>	 dan-nl: They are really the same directory on OS X
[19:55:54] <bd808>	 The file path that came out of the first patch would have been literally 'URL'
[19:56:11] <bd808>	 Which is totally not what I meant to happen
[19:59:34] <dan-nl>	 right, saw that as well … verified … it works now :)
[20:01:05] <dan-nl>	 bd808 guess i need to wait for it to get onto beta before i can run another test
[20:01:14] <bd808>	 Yeah. Sorry about that.
[20:01:27] <dan-nl>	 earlier test of gwtoolset worked fine … no orphaned URLxxx files, no exceptions
[20:01:41] <dan-nl>	 i think we're ready for tuesday … 
[20:01:52] <dan-nl>	 but i don't know about that production config
[20:02:40] <bd808>	 Poke Greg and see if he can get some folks behind validating it.
[20:02:56] <dan-nl>	 k
[20:04:29] <bd808>	 I just self-merged that patch to unblock you in beta
[20:04:36] <dan-nl>	 how does TempFSFile auto-clean-up the temp file differently than what currently happens?
[20:04:39] <dan-nl>	 ah, thanks!
[20:05:01] <dan-nl>	 ah i see bind
[20:05:57] <dan-nl>	 although do you need to run auto collect first to set the canDelete flag?
[20:06:18] <bd808>	 TempFSFile has a destructor that cleans up. So you bind it to the object that needs the file and when that bound object goes out of scope it will release the TempFSFile and it will in turn ensure that the temp file is deleted
[20:06:48] * bd808  thought that was default...

[20:06:51] <dan-nl>	 right, but it looks like the destructor checks canDelete
[20:07:09] * dan-nl  looking at the factory

[20:07:28] <bd808>	 grrr… you're right
[20:07:31] <dan-nl>	 ja, the factory sets the flag to true
[20:07:44] <dan-nl>	 line 65
[20:08:29] <bd808>	 ok. Time for me to eat something
[20:08:41] <dan-nl>	 so it looks like it's okay
[20:08:45] <dan-nl>	 k, have a good lunch
[20:08:51] <dan-nl>	 thanks for the patch!
[20:09:11] <grrrit-wm>	 (03CR) 10jenkins-bot: [V: 04-1] Localisation updates from https://translatewiki.net. [extensions/MultimediaViewer] - 10https://gerrit.wikimedia.org/r/101392 (owner: 10L10n-bot)
[20:09:16] <marktraceur>	 Huh.
[20:09:19] <marktraceur>	 That's funky.
[20:09:19] <bd808|LUNCH>	 Fixing what I broke was the least I could do
[20:10:54] <grrrit-wm>	 (03CR) 10MarkTraceur: "recheck" [extensions/MultimediaViewer] - 10https://gerrit.wikimedia.org/r/101392 (owner: 10L10n-bot)
[20:13:26] <grrrit-wm>	 (03CR) 10MarkTraceur: [C: 04-1] "Spacing + thoughts about mlb.css" (036 comments) [extensions/MultimediaViewer] - 10https://gerrit.wikimedia.org/r/100362 (owner: 10Pginer)
[20:14:24] <grrrit-wm>	 (03PS2) 10MarkTraceur: Localisation updates from https://translatewiki.net. [extensions/MultimediaViewer] - 10https://gerrit.wikimedia.org/r/101392 (owner: 10L10n-bot)
[20:15:44] <grrrit-wm>	 (03CR) 10Raimond Spekking: [C: 032 V: 032] Localisation updates from https://translatewiki.net. Change-Id: Ie966a7edb3560efdfa2931f501bdde535e364bc5 [extensions/MultimediaViewer] - 10https://gerrit.wikimedia.org/r/101392 (owner: 10L10n-bot)
[20:17:52] <grrrit-wm>	 (03CR) 10MarkTraceur: [C: 04-1] Say 'No description' if the description is empty If the image description is empty, then it displays No Description Bug 56446 (033 comments) [extensions/MultimediaViewer] - 10https://gerrit.wikimedia.org/r/100207 (owner: 10Anirudh)
[20:18:12] <marktraceur>	 No
[20:18:13] <marktraceur>	 Please
[20:18:18] <marktraceur>	 Steamroll over jenkins
[20:19:44] <grrrit-wm>	 (03CR) 10MarkTraceur: [C: 04-1] "? what is mw.data? I've never seen it before and it seems undefined on my local wiki. This can't have worked when you tested locally." [extensions/MultimediaViewer] - 10https://gerrit.wikimedia.org/r/99533 (owner: 10Yamelnychuk)
[20:28:11] <grrrit-wm>	 (03CR) 10Anomie: Rewrite template parsing with DOMDocument (031 comment) [extensions/CommonsMetadata] - 10https://gerrit.wikimedia.org/r/100569 (owner: 10Gergő Tisza)
[21:31:13] <marktraceur>	 tgr: The phpunit tests for CMD look like they don't have any labels
[21:31:16] <marktraceur>	 Which is mostly cool
[21:31:48] <tgr>	 what do you mean by labels?
[21:32:04] <marktraceur>	 tgr: A final argument to the assertion
[21:32:09] <marktraceur>	 It's like, the title of the test.
[21:32:19] <marktraceur>	 It gives the programmer a better hint as to what broke
[21:32:23] <tgr>	 ah
[21:32:24] <marktraceur>	 I'm just remarking on it
[21:32:31] <tgr>	 i tend to avoid those
[21:32:55] <tgr>	 i think recent phpunit versions give less information when there is a user-defined message
[21:33:15] <tgr>	 and phpunit has pretty decent error messages most of the time
[21:33:23] <marktraceur>	 'kay
[21:33:36] <tgr>	 admittedly, my custum assert methods dont
[21:34:00] <tgr>	 i was too lazy to create assertion classes
[21:34:18] <tgr>	 but i can do that if you prefer
[21:34:26] <marktraceur>	 Naw, it's OK
[21:38:48] <grrrit-wm>	 (03CR) 10Nemo bis: "Hi, thanks for your patch nonetheless. Next time, to make a nice bot do some dirty work for you etc., follow <https://www.mediawiki.org/wi" [extensions/MultimediaViewer] - 10https://gerrit.wikimedia.org/r/99366 (owner: 10Durga94)
[21:40:55] <grrrit-wm>	 (03CR) 10MarkTraceur: "Instead, you should add a message called multimediaviewer-repository-local and put the text in that, then conditionally use the message in" [extensions/MultimediaViewer] - 10https://gerrit.wikimedia.org/r/99366 (owner: 10Durga94)
[21:43:19] <dan-nl>	 hey guys, i just finished a 2nd test on beta that worked as expected. i think everything is ready for the tuesday deploy. thanks again for all your help, i really appreciate it! http://commons.wikimedia.beta.wmflabs.org/w/index.php?title=Category:GWToolset_Batch_Upload
[21:52:54] <bd808>	 dan-nl: \o/ I want to see it shipped just so you stop giving me things to review. :)
[21:53:22] <dan-nl>	 lol
[21:54:47] <marktraceur>	 bd808: It won't stop, he'll just start asking for review *and then* for you to backport and deploy things
[21:54:55] <marktraceur>	 You can't win, man
[21:55:30] <dan-nl>	 oh cool thanks for volunteering marktraceur … bd808 you're off the hook ;)
[21:55:54] <bd808>	 It will be a multimedia team concern as I recall
[21:55:57] <marktraceur>	 Aaaaauuuuuuggggghhhhhh
[21:56:03] <marktraceur>	 No, yeah, totally
[21:56:12] * marktraceur  uses conflicting language

[21:56:31] <marktraceur>	 I'll happily shove your code into production.
[21:56:44] <bd808>	 Think of the pretty pictures for media viewer
[21:56:54] <marktraceur>	 Heh
[21:59:19] <grrrit-wm>	 (03CR) 10MarkTraceur: [C: 032] "Yay tests. One small tweak you could make inline, but it's not a blocker." (031 comment) [extensions/CommonsMetadata] - 10https://gerrit.wikimedia.org/r/98546 (owner: 10Gergő Tisza)
[21:59:21] <marktraceur>	 Whew.
[21:59:26] <marktraceur>	 CMD is complicated shit.
[21:59:32] <grrrit-wm>	 (03Merged) 10jenkins-bot: Add tests for InformationParser [extensions/CommonsMetadata] - 10https://gerrit.wikimedia.org/r/98546 (owner: 10Gergő Tisza)
[22:01:33] <marktraceur>	 +888, -853 <-- ahhhhhhhhhhhhhhhhhhhhhhhhh
[22:03:00] <marktraceur>	 I guess most of it is moving things. OK.
[22:05:35] <dan-nl>	 off to the weekend; enjoy yours as well
[22:05:49] <marktraceur>	 Cheers dan-nl 
[22:31:12] <grrrit-wm>	 (03Abandoned) 10Yamelnychuk: Included license in the title attribute of the link in "use this file" output. [extensions/MultimediaViewer] - 10https://gerrit.wikimedia.org/r/99533 (owner: 10Yamelnychuk)
[23:05:38] <grrrit-wm>	 (03CR) 10Gergő Tisza: Add tests for InformationParser (031 comment) [extensions/CommonsMetadata] - 10https://gerrit.wikimedia.org/r/98546 (owner: 10Gergő Tisza)
[23:45:37] <grrrit-wm>	 (03CR) 10MarkTraceur: [C: 04-1] "Minor issue with a comment." (032 comments) [extensions/CommonsMetadata] - 10https://gerrit.wikimedia.org/r/100181 (owner: 10Gergő Tisza)