[08:33:18] heloo [16:11:38] New patchset: Hashar; ".gitreview file" [integration/doc] (master) - https://gerrit.wikimedia.org/r/34889 [16:11:38] New patchset: Hashar; "CI workflow using GraphViz" [integration/doc] (master) - https://gerrit.wikimedia.org/r/34890 [16:12:28] Change merged: Hashar; [integration/doc] (master) - https://gerrit.wikimedia.org/r/34889 [16:16:41] New review: Hashar; "Imported picture on commons at https://commons.wikimedia.org/wiki/File:Wikimedia_CI_workflow.svg" [integration/doc] (master); V: 0 C: 0; - https://gerrit.wikimedia.org/r/34890 [16:21:15] New review: Hashar; "Letsss do it :-)" [integration/grunt-contrib-wikimedia] (master); V: 0 C: 2; - https://gerrit.wikimedia.org/r/34488 [16:21:22] New review: Hashar; "Letsss do it :-)" [integration/grunt-contrib-wikimedia] (master); V: 1 C: 2; - https://gerrit.wikimedia.org/r/34488 [16:21:23] Change merged: Hashar; [integration/grunt-contrib-wikimedia] (master) - https://gerrit.wikimedia.org/r/34488 [16:24:12] New patchset: Hashar; "update grunt-contrib-wikimedia" [integration/jenkins] (master) - https://gerrit.wikimedia.org/r/34892 [16:24:28] New review: Hashar; "Jenkins updated with https://gerrit.wikimedia.org/r/34892" [integration/grunt-contrib-wikimedia] (master) - https://gerrit.wikimedia.org/r/34488 [16:24:39] Change merged: Hashar; [integration/jenkins] (master) - https://gerrit.wikimedia.org/r/34892 [16:30:47] New patchset: Hashar; "ignore jobs generated by JJB and stop tracking them" [integration/jenkins] (master) - https://gerrit.wikimedia.org/r/34899 [16:31:03] Change merged: Hashar; [integration/jenkins] (master) - https://gerrit.wikimedia.org/r/34899 [17:27:28] New patchset: Hashar; "WMF wrapper around grunt" [integration/jenkins] (master) - https://gerrit.wikimedia.org/r/34720 [17:27:53] New review: Hashar; "PS2: add some explanation about cryptic shell command." [integration/jenkins] (master); V: 0 C: 0; - https://gerrit.wikimedia.org/r/34720 [17:39:00] humm humm, zomgsecret security fixes are flooding teh logs! :P [18:18:08] /win 14 [18:43:49] New patchset: Nemo bis; "Remove Wikimedia-l from counts where not relevant" [analytics/wikistats] (master) - https://gerrit.wikimedia.org/r/34907 [18:45:42] New patchset: Nemo bis; "Remove Wikimedia-l from counts where not relevant" [analytics/wikistats] (master) - https://gerrit.wikimedia.org/r/34907 [19:56:04] aaadddaadwqdqwdwefewfewfwefss [19:56:41] Nikerabbit: +2 [19:56:47] sorry about that, didn't mean to press enter, the connection is jsut very shake [19:57:08] can take 10 minutes before any traffics goes in or out [21:36:49] Hello. [21:37:24] marktraceur: have you already had a contact with Sucheta Ghoshal about EtherEditor? [22:38:07] oh, 683 open commits across all repos [22:38:28] 473 without -1 or -2 [22:54:15] just today I -1ed a bunch [22:55:26] "unanswered question" is not really a valid reason to -1 a patch, Nikerabbit. [22:58:17] Krenair: either I give it -1 or don't review it at all. which one do you prefer? [22:59:42] Don't review it at all if your -1 is going to be about some unimportant question not being answered [23:03:56] Krenair: which commit are you talking about? [23:04:26] https://gerrit.wikimedia.org/r/#/c/33878/ [23:06:08] they clutter the dashboard otherwise, AFAIK [23:07:50] https://gerrit.wikimedia.org/r/#/dashboard/93 [23:08:52] I had to do something, and given that the answer is relevant to code review, -1 seemed very sensible [23:10:38] is the sort order of the review requested section documented somewhere? [23:14:20] date of first submission [23:18:50] thanks [23:23:49] Nikerabbit: Given that you +2'ed https://gerrit.wikimedia.org/r/#/c/30173/ which was agreed to wait until we'd come up with a socialisation plan, I assume you're going to be doing that work now? [23:24:25] James_F: hmm, no [23:25:05] Nikerabbit: ... so who do you think is going to do it? It was waiting on me being less busy, but you chose to +2 it about a month early in that case. :-) [23:25:25] Nikerabbit: Not all discussion about commits is done on gerrit, you know. :-) [23:25:33] James_F: I think you are confusing +2 with merge [23:26:12] perhaps you should give -2 to it just in case [23:26:22] James_F, if there's something extra an approver/merger should know, it must be made clear on Gerrit [23:26:28] Nikerabbit: All dozen of them that Siebrand did? [23:26:44] James_F: I don't know what you are talking about [23:27:09] Nikerabbit: There was quite a long discussion about this on ... the WMF engineering list, I think. [23:27:20] ah, you mean the removal of pref X commits? [23:27:26] Nikerabbit: Yes. [23:27:40] Nikerabbit: Erik asked us to 'do it properly'. [23:29:14] * James_F will go and -2 them all then. [23:30:04] frankly I see that as WMF holding back MW development [23:31:19] but there is a practical problem, either I give a review or I don't give a review and don't get any updates on the discussion [23:32:37] Why would you +2 but not merge? [23:32:43] Be careful with getting used to that. [23:32:49] Nikerabbit: Comment added. [23:32:53] Not so long from now you won't be able to +2 without doing submit. [23:33:16] as it will auto merge after +2 and jenkins passes [23:33:18] Nikerabbit: I think it's disrespectful of our users to just yank settings from the software without even notifying them. Do you disagree? [23:36:45] Krinkle: that sounds nasty. How do I indicate "this is good but should only be merged when it is going to be deployed"? [23:37:08] Nikerabbit: +1 I guess, or -2 for "Don't merge" [23:37:41] James_F: nope, but I do not necessary agree on the ways or extend for that [23:37:46] Nikerabbit: Besides, if you are thinking that, you're probably doing something wrong. [23:37:56] Krinkle: or "the code looks okay, but I haven't verified it myself yet" [23:38:01] i.e. relying on something you shouldn't be relying. [23:38:33] Krinkle: I regularly do that for wmf config changes that I'm planning to deploy during deployment windows [23:38:38] if it needs care in deploy, maybe it needs a configuration toggle. if it needs something else, it needs that. whatever the case +2 means Merge this now! [23:38:49] Nikerabbit: I'm certainly not saying that we should make most changes a bureaucratic nightmare. :-) [23:39:05] Nikerabbit: Then I highly recommend stopping that and discuss it and come up with a more collaborative and documented workflow [23:39:25] If I see +2 somewhere without submit, I often just merge it if I agree as well (unless a comment gives reason to do otherwise) [23:39:37] Nikerabbit: If you plan to look at it later, look at it later and don't leave a review. [23:40:09] Krinkle: have you seen https://gerrit.wikimedia.org/r/#/dashboard/93 ? [23:40:20] don't care. [23:40:27] ? [23:40:29] you are essentially asking me to keep external list of commits I want to deploy [23:40:50] No, I'm asking you to dot do things you don't mean to, relying on undocumented workflows. [23:41:06] If you +2 it means merge this, now. The only reason not to submit is waiting for unit tests to pass. [23:41:37] Whatever you do, I don't care. But this can't it, at least not now (feel free to propose it though). [23:42:07] yes you are: -2 would mean nobody else can submit it, -1 would mean it needs fixing, 0 means I can't find it, +1 would mean I want somebody else to do it and +2 I'm not allowed to use [23:42:51] that's not exactly true, I can't find it either way [23:42:56] Nikerabbit: Give me an example of where you clicked a change, have reviewed it, find it ready for deployment and then don't? [23:43:33] https://gerrit.wikimedia.org/r/#/c/34300/ [23:44:07] (I'm not disagreeing), why not deploy it right now? [23:44:51] Nikerabbit: [23:44:57] because it is middle of the night and I usually send announcement to mediawiki-i18n list of changes I will deploy a day ahead [23:46:06] Nikerabbit: What's the problem with them being merged into master before they're deployed? [23:46:09] In my workflow I would -1 it because it needs an announcement to -18n before merging (merge==deploy effectively). [23:46:30] James_F: Because merge=deploy, if you merge but have reasons for it not being allowed to be deployed yet, that's even worse [23:46:33] e.g. git pull, scap [23:46:40] Krinkle: Merge!=deploy. [23:46:55] Krinkle: Or are you talking about live config changes? [23:46:57] We're talking about merging into *master*, not wmf/1.21wmfN, so merge!=deploy [23:46:58] * James_F is confused. [23:46:58] James_F: If you merge and must not deploy, then you need to stop what you're doing. [23:47:12] RoanKattouw: this is mediawiki-config [23:47:15] Ooooh [23:47:23] Yeah mw-config is different [23:47:26] the most annoying thing I know is that someone has merged stuff but not deployed, and then it is on my face when I want to deploy my changes only [23:47:31] merge==deploy for mw-config [23:47:39] Or at least it damn well should be, because ... what Nike said [23:47:44] Dereckson: Yes, I've talked to her, she should have an intro task already [23:48:22] So... Why not merge each of the wanted changes into a new branch, and then merge that into master as the deploy? [23:48:47] That seems overly complicated, unless you're regularly deploying sets of >3 changes at a scheduled time [23:48:49] Leaving the branch-merge as a -2 until go-live? [23:49:11] RoanKattouw: Nikerabbit said he didn't want to keep the list of changes in his head or elsewise off-gerrit. This is a way to keep it in gerrit. [23:49:12] Nikerabbit: So, anyhow. There are many options. You could -1 for needing announcement first, and star is as a reminder. But whatever the case, do not +2 if you don't it deployed yet just so that you can use "reviewer:Nike CR+2" query as a personal todo list. [23:49:19] Right that makes sense [23:49:28] -2ing a merge commit makes sense [23:49:32] Krinkle: got it [23:50:03] Nikerabbit: I'm happy to help find a better solution, just wanted to say that +2 will soon be auto-merge. And even if not, I imagine it might be confusing to others. [23:50:24] -2 would stop everyone else from merging it either, so for now I'm just using +1 + gerrit comment [23:50:55] You could -2, announcements are important too :) [23:51:21] but I can understand if you don't want the extra bureaucracy of needing you to undo -2 once announced [23:51:37] marktraceur: ok (we exchanged some mails, and I extracted the collaborative aspect from her first proposal - she wanted to use the Google Translate Toolkit to help the article translation workflow on WP) [23:51:43] it depends, delaying stuff just for the sake of doing it during commit window is sometimes excessive [23:51:58] s/commit/deployment/ [23:52:13] Dereckson: Yeah, I got the translation bit from her at first and thought she might have wanted aharoni, but she's on it now [23:52:30] I rarely deploy outside my window, but I'm usually fine if someone else wants do it [23:52:43] hence +2 [23:53:47] good night [23:57:18] any devs around?