|
2019-01-30 14:08:14
|
<snitch>
|
[[Tech]]; 129.0.210.246; ABBASALI; https://meta.wikimedia.org/w/index.php?diff=18829457&oldid=18812474&rcid=13116047
|
|
2019-01-30 14:10:31
|
<snitch>
|
[[Tech]]; Defender; Reverted changes by [[Special:Contributions/129.0.210.246|129.0.210.246]] ([[User talk:129.0.210.246|talk]]) to last version by ArchiverBot; https://meta.wikimedia.org/w/index.php?diff=18829463&oldid=18829457&rcid=13116055
|
|
2019-01-30 15:00:16
|
<wm-bot>
|
Technical Advice IRC meeting starting in 60 minutes in channel #wikimedia-tech, hosts: @amir1 & @Thiemo_WMDE - all questions welcome, more infos: https://www.mediawiki.org/wiki/Technical_Advice_IRC_Meeting
|
|
2019-01-30 15:00:26
|
<Amir1>
|
Hello there o/
|
|
2019-01-30 15:50:16
|
<wm-bot>
|
Technical Advice IRC meeting starting in 10 minutes in channel #wikimedia-tech, hosts: @amir1 & @Thiemo_WMDE - all questions welcome, more infos: https://www.mediawiki.org/wiki/Technical_Advice_IRC_Meeting
|
|
2019-01-30 16:01:49
|
<Amir1>
|
o/
|
|
2019-01-30 16:02:07
|
<xSavitar>
|
o/
|
|
2019-01-30 16:02:17
|
<Thiemo_WMDE>
|
Hello tech people.
|
|
2019-01-30 16:02:19
|
<Amir1>
|
Hello and welcome to TAIM. Thiemo_WMDE and I are your hosts today
|
|
2019-01-30 16:03:18
|
<Amir1>
|
xSavitar: o/
|
|
2019-01-30 16:05:07
|
<xSavitar>
|
Q1. Can one escape using global $IP like other globals such as $wgVersion? One way is using Config to get the values of such but I'm wondering how that can be done with $IP. Any ideas please?
|
|
2019-01-30 16:05:44
|
<Thiemo_WMDE>
|
According to https://www.mediawiki.org/wiki/Technical_Advice_IRC_Meeting we don't have questions prepared. Today is your chance to ask all the ORES questions you might have, and everything else.
|
|
2019-01-30 16:06:04
|
<Amir1>
|
xSavitar: Mostly services
|
|
2019-01-30 16:06:10
|
<Amir1>
|
let me show an example
|
|
2019-01-30 16:06:17
|
<xSavitar>
|
Okay, nice! :)
|
|
2019-01-30 16:06:18
|
<Amir1>
|
(and context object)
|
|
2019-01-30 16:07:42
|
<Amir1>
|
xSavitar: https://github.com/wikimedia/mediawiki-extensions-Cognate/blob/master/src/ServiceWiring.php#L35
|
|
2019-01-30 16:07:55
|
<xSavitar>
|
While you're getting the example, I've used MediaWikiServices to get $wgParser that was deprecated in 1.32
|
|
2019-01-30 16:07:57
|
<Amir1>
|
This is one example of getting the config from services
|
|
2019-01-30 16:08:12
|
<xSavitar>
|
Okay!
|
|
2019-01-30 16:08:58
|
<Amir1>
|
The other option is get the context that is injected to the class (e.g. an API module, Special page, etc.)
|
|
2019-01-30 16:09:08
|
<xSavitar>
|
So Amir1, from that example you just shared, one needs to write a service for getting $IP right? Or does one already exist?
|
|
2019-01-30 16:09:51
|
<xSavitar>
|
Amir1: For the second option, you mean something like RequestContext::getMain()->getConfig()->get(...) right?
|
|
2019-01-30 16:10:21
|
<Thiemo_WMDE>
|
xSavitar: Sorry to ask, but what is the issue you want to solve? As far as I can tell it's fine to use the global $IP.
|
|
2019-01-30 16:11:07
|
<xSavitar>
|
Thiemo_WMDE: I'm working on this currently: https://gerrit.wikimedia.org/r/c/mediawiki/core/+/487057
|
|
2019-01-30 16:11:14
|
<Amir1>
|
It should be used like this: MediawikiServices::getInstance()->getMainConfig()->get( 'CognateCluster' );
|
|
2019-01-30 16:11:26
|
<Amir1>
|
xSavitar: This would give you the config value
|
|
2019-01-30 16:11:32
|
<xSavitar>
|
And yes, global $IP is fine but I'm just wondering if there are better alternatives apart from using global
|
|
2019-01-30 16:11:43
|
<Amir1>
|
it's better to inject the "mainConfig" object though but it's optional
|
|
2019-01-30 16:12:28
|
<Amir1>
|
xSavitar: Can I see the gerrit patch?
|
|
2019-01-30 16:12:31
|
<Amir1>
|
I might have an idea
|
|
2019-01-30 16:12:39
|
<xSavitar>
|
Amir1: Yeah, this is it: https://gerrit.wikimedia.org/r/c/mediawiki/core/+/487057
|
|
2019-01-30 16:13:02
|
<xSavitar>
|
Amir1: Thanks! Using services is fine by me or even context.
|
|
2019-01-30 16:13:21
|
<Amir1>
|
xSavitar: In that case it's better to use the context injected to the special page class
|
|
2019-01-30 16:13:35
|
<Amir1>
|
replace RequestContext::getMain()->getConfig(); with $this->context->getConfig()
|
|
2019-01-30 16:14:17
|
<Amir1>
|
s/$this->context/$this->getContext()
|
|
2019-01-30 16:14:48
|
<xSavitar>
|
Okay! $this->getContext()->getConfig() should do the trick you mean?
|
|
2019-01-30 16:15:03
|
<Amir1>
|
yup
|
|
2019-01-30 16:15:24
|
<xSavitar>
|
on top of that, ->get( 'IP' )?
|
|
2019-01-30 16:15:31
|
<Amir1>
|
but regarding $IP is not part of config
|
|
2019-01-30 16:15:36
|
<xSavitar>
|
Exactly
|
|
2019-01-30 16:15:39
|
<Amir1>
|
it's not accessible
|
|
2019-01-30 16:15:48
|
<xSavitar>
|
I tried it and it didn't work. You're perfectly correct!
|
|
2019-01-30 16:16:06
|
<xSavitar>
|
Maybe a service can be introduced for that?
|
|
2019-01-30 16:16:15
|
<Amir1>
|
I don't have any better alternative for $IP, maybe we can define a config variable in a central place
|
|
2019-01-30 16:16:22
|
<Thiemo_WMDE>
|
xSavitar: I would suggest to not touch any code using $IP for the patch you are currently working on. Focus on the other stuff first.
|
|
2019-01-30 16:16:23
|
<xSavitar>
|
Or just use global $IP for this special case as Thiemo_WMDE suggested?
|
|
2019-01-30 16:16:34
|
<Amir1>
|
it's very speciall
|
|
2019-01-30 16:16:37
|
<Amir1>
|
*special
|
|
2019-01-30 16:17:04
|
<xSavitar>
|
You're right! That is why I've been dealing with other issues and left $IP as-is
|
|
2019-01-30 16:17:19
|
<xSavitar>
|
I wanted clues and better ideas in the meeting before moving ahead with the patch
|
|
2019-01-30 16:17:30
|
<xSavitar>
|
So in that case, global $IP is fine ATM
|
|
2019-01-30 16:17:59
|
<Amir1>
|
It's not fine (no global is fine :D) but we don't have a good alternative for now
|
|
2019-01-30 16:18:10
|
<xSavitar>
|
Okay!
|
|
2019-01-30 16:18:13
|
<xSavitar>
|
:)
|
|
2019-01-30 16:20:24
|
<xSavitar>
|
Q2. How does one know following the deprecation policy here: https://www.mediawiki.org/wiki/Deprecation_policy when a deprecated variable or method or class should be finally removed
|
|
2019-01-30 16:20:44
|
<xSavitar>
|
Sometimes I see variables, methods etc saying they have been deprecated from mw 1.xx
|
|
2019-01-30 16:20:57
|
<xSavitar>
|
But doesn't state which version of mw that method or var etc should be removed
|
|
2019-01-30 16:21:21
|
<xSavitar>
|
I've seen some cases that explicitly say that but rare. So I'm wondering if that should be highlighted too?
|
|
2019-01-30 16:21:29
|
<xSavitar>
|
Just a suggestion, not really a technical question per say
|
|
2019-01-30 16:22:24
|
<xSavitar>
|
Amir1, Thiemo_WMDE, ^^
|
|
2019-01-30 16:22:28
|
<Thiemo_WMDE>
|
Deprecation is just a marker, but doesn't actually change the code. Removal on the other hand will break stuff when it is still used, but gone. Usually this means we remove stuff only when we are sure it is not used any more (according to our codesearch).
|
|
2019-01-30 16:23:36
|
<Thiemo_WMDE>
|
My personal opinion is: having a deprecation sit there for a long time is not a big issue. It's important to not introduce new usages of deprecated code. But actually removing it can wait.
|
|
2019-01-30 16:23:42
|
<Amir1>
|
xSavitar: we usually let one release pass after removing a deprecated functionality so things that are deprecated in 1.32 and 1.33 should better stay
|
|
2019-01-30 16:24:19
|
<Thiemo_WMDE>
|
Letting 1 release pass is the minimum. But it can be dragged for many, many versions when it's hard to remove/replace usages.
|
|
2019-01-30 16:24:38
|
<xSavitar>
|
Cool!
|
|
2019-01-30 16:24:51
|
<xSavitar>
|
Also assuming that some extensions don't have very active contributors as others
|
|
2019-01-30 16:25:03
|
<xSavitar>
|
So some extensions using such could be lagging behind
|
|
2019-01-30 16:25:19
|
<xSavitar>
|
And removing those could break the usage of those extensions
|
|
2019-01-30 16:25:23
|
<Amir1>
|
xSavitar: To be fair, we only care about WMF-deployed extensions which they should have a maintainer
|
|
2019-01-30 16:25:26
|
<xSavitar>
|
Makes a lot of sense, thanks thanks
|
|
2019-01-30 16:25:38
|
<xSavitar>
|
Oh okay!
|
|
2019-01-30 16:25:38
|
<Amir1>
|
others would just follow the release cycle
|
|
2019-01-30 16:25:55
|
<xSavitar>
|
Alright, nice!
|
|
2019-01-30 16:26:41
|
<xSavitar>
|
Amir1: But you know sometimes it's not also easy to follow the release cycle
|
|
2019-01-30 16:26:56
|
<xSavitar>
|
Especially when things are moving pretty fast
|
|
2019-01-30 16:27:11
|
<xSavitar>
|
On one hand and catching up is slow on the other
|
|
2019-01-30 16:27:22
|
<Amir1>
|
yeah
|
|
2019-01-30 16:27:44
|
<xSavitar>
|
But generally I get the point. Thanks!
|
|
2019-01-30 16:27:57
|
<Amir1>
|
There are things that make changes easier, like avoid using globals and such (standard software engineering practices)
|
|
2019-01-30 16:28:14
|
<Amir1>
|
but they are not followed in lots of extensions
|
|
2019-01-30 16:28:41
|
<xSavitar>
|
Yeah!
|
|
2019-01-30 16:28:41
|
<Thiemo_WMDE>
|
xSavitar: I tend to think of deprecated code as non-existing. We should act like it is not there, not use it, and replace usages when we can. Some day we will realize it is not used any more.
|
|
2019-01-30 16:29:18
|
<xSavitar>
|
Thiemo_WMDE: Cool! :)
|
|
2019-01-30 16:29:42
|
<xSavitar>
|
Meaning patches coming in with deprecated usage shouldn't be adviced to be merged right?
|
|
2019-01-30 16:29:48
|
<xSavitar>
|
As it's causing more harm than good in the long run
|
|
2019-01-30 16:30:24
|
<Amir1>
|
xSavitar: They ones that introduce new usages on deprecated functionalities definitely should get a -1
|
|
2019-01-30 16:30:53
|
<xSavitar>
|
jots down this review tip!
|
|
2019-01-30 16:31:59
|
<Thiemo_WMDE>
|
xSavitar: As always, the answer is "it depends". ;-) Usually it's worth a -1, but only if a replacement exists. Sometimes it doesn't.
|
|
2019-01-30 16:32:57
|
<xSavitar>
|
Thiemo_WMDE: Cool! That raises another question. Why should something be deprecated if one isn't working or there isn't a replace for it on board?
|
|
2019-01-30 16:33:09
|
<xSavitar>
|
*replacement
|
|
2019-01-30 16:33:52
|
<xSavitar>
|
Or maybe it's the kind of deprecation whose future doesn't want to be supported anymore?
|
|
2019-01-30 16:34:09
|
<xSavitar>
|
Like this is the last version of xyz and we are removing this support from core etc
|
|
2019-01-30 16:34:21
|
<Amir1>
|
it can have lots of reasons
|
|
2019-01-30 16:34:38
|
<xSavitar>
|
Okay!
|
|
2019-01-30 16:34:52
|
<Amir1>
|
it can be there to address technical debt (e.g. we recently split a huge class to several small ones)
|
|
2019-01-30 16:35:06
|
<xSavitar>
|
So in that case, one should double check before dropping a -1 as Thiemo_WMDE mentioned then :)
|
|
2019-01-30 16:35:07
|
<Amir1>
|
the main class got deprecated
|
|
2019-01-30 16:35:12
|
<Amir1>
|
yup
|
|
2019-01-30 16:35:28
|
<xSavitar>
|
Oh nice! That example is a good illustration point. Thanks
|
|
2019-01-30 16:36:31
|
<xSavitar>
|
Q3. I had a weird scenario today on this patch!
|
|
2019-01-30 16:36:37
|
<xSavitar>
|
https://gerrit.wikimedia.org/r/c/mediawiki/extensions/Renameuser/+/487134
|
|
2019-01-30 16:37:07
|
<xSavitar>
|
I and Thiemo_WMDE have a question (right Thiemo)?
|
|
2019-01-30 16:37:36
|
<xSavitar>
|
So the idea is a call to Title::moveTo() passes the reason with $this->msg() causes phan to throw a warning
|
|
2019-01-30 16:37:55
|
<Amir1>
|
one thing: The hook handlers should not return any value (that got deprecated :P)
|
|
2019-01-30 16:37:56
|
<xSavitar>
|
But assigning the value of $this->msg() to a variable doesn't, now sure why
|
|
2019-01-30 16:39:49
|
<xSavitar>
|
Amir1: Ohhhh yeah! Needs to be removed
|
|
2019-01-30 16:39:57
|
<xSavitar>
|
I'll update that patch soon :), thanks very much!
|
|
2019-01-30 16:40:31
|
<Amir1>
|
Plus, styling convention, one argument per line when is too big
|
|
2019-01-30 16:41:03
|
<xSavitar>
|
:)
|
|
2019-01-30 16:41:18
|
<Amir1>
|
the big reason is php is stupid and unlike python doesn't have named arguments, so it's better 1- not to have too many arguments 2- make sure it's very visible
|
|
2019-01-30 16:41:45
|
<Amir1>
|
if you call the function in wrong order things break (one month ago I caused data corruption because of that)
|
|
2019-01-30 16:42:05
|
<Amir1>
|
the good thing was it was just a vandalism that went on the wrong revision :D
|
|
2019-01-30 16:43:00
|
<xSavitar>
|
Amir1: Wow, I've been in such a situation. Recently one of my patches broke production and translatewiki.net due to type hints :(
|
|
2019-01-30 16:43:04
|
<xSavitar>
|
I'm so ashamed :(
|
|
2019-01-30 16:43:28
|
<Amir1>
|
I've done that twice
|
|
2019-01-30 16:44:06
|
<Amir1>
|
don't be ashamed, when you get things done, breaking things is inevitable
|
|
2019-01-30 16:44:37
|
<Amir1>
|
IMHO, it's a good sign
|
|
2019-01-30 16:45:17
|
<xSavitar>
|
Hmmm... I honestly pray I shouldn't do that again. I need to be more care next time and check and double check to always be sure.
|
|
2019-01-30 16:45:29
|
<xSavitar>
|
Though, there is really no guarantee
|
|
2019-01-30 16:45:43
|
<Amir1>
|
that's the point of breaking things, we learn not to them again
|
|
2019-01-30 16:46:04
|
<Amir1>
|
(which apparently I didn't and did it twice)
|
|
2019-01-30 16:46:25
|
<xSavitar>
|
Amir1: I've updated this patch: https://gerrit.wikimedia.org/r/c/mediawiki/extensions/Renameuser/+/487134 and looped in your feedback + that of Thiemo_WMDE :)
|
|
2019-01-30 16:46:26
|
<Amir1>
|
regarding you fun phan issue. I don't know much about phan, Thiemo_WMDE is the expert here
|
|
2019-01-30 16:46:38
|
<Amir1>
|
s/you/your
|
|
2019-01-30 16:47:03
|
<xSavitar>
|
Okay, thanks!
|
|
2019-01-30 16:48:55
|
<xSavitar>
|
Yours is really scary Amir1, I'm still thinking about it now
|
|
2019-01-30 16:49:11
|
<xSavitar>
|
I'm really afraid of such issues
|
|
2019-01-30 16:49:57
|
<Amir1>
|
:D
|
|
2019-01-30 16:50:17
|
<xSavitar>
|
My last question that just came up in mind now: Q4: I think mediawiki-docker is meant just for production right? or testing but not for development?
|
|
2019-01-30 16:50:31
|
<xSavitar>
|
Maybe addshore has a word here? Not sure!
|
|
2019-01-30 16:50:34
|
<Thiemo_WMDE>
|
Just in case, does somebody else have a question? ;-)
|
|
2019-01-30 16:51:37
|
<xSavitar>
|
Please tech people, we've got some bigs brains hosting, ask your questions! :)
|
|
2019-01-30 16:52:34
|
<xSavitar>
|
s/bigs/big
|
|
2019-01-30 16:53:12
|
<xSavitar>
|
Thiemo_WMDE: that phan thing seems to be a false positive or kinda flaky
|
|
2019-01-30 16:53:26
|
<Amir1>
|
xSavitar: regarding your docker question
|
|
2019-01-30 16:53:28
|
<xSavitar>
|
Just got the warning again but this time with the variable
|
|
2019-01-30 16:53:40
|
<Amir1>
|
the plan is to have docker in prod in one or two years
|
|
2019-01-30 16:53:44
|
<Amir1>
|
as part of SSDD
|
|
2019-01-30 16:53:51
|
<Amir1>
|
https://wikitech.wikimedia.org/wiki/Streamlined_Service_Delivery_Design
|
|
2019-01-30 16:54:16
|
<Amir1>
|
but CI uses docker to test
|
|
2019-01-30 16:54:41
|
<Amir1>
|
it's always easier to use docker to develop and push changed to gerrit once you achieved what you want
|
|
2019-01-30 16:55:09
|
<Amir1>
|
(because it's self-contained and easy to set up)
|
|
2019-01-30 16:55:23
|
<xSavitar>
|
Okay! Cool!
|
|
2019-01-30 16:55:40
|
<Amir1>
|
and at the end, in gerrit, it doesn't matter if you used vagrant or docker or native installation
|
|
2019-01-30 16:56:25
|
<xSavitar>
|
:)
|
|
2019-01-30 17:00:42
|
<Amir1>
|
With no questions and no time left, I conclude this TAIM. Have a nice evening/morning/whatever everyone!
|
|
2019-01-30 17:01:04
|
<xSavitar>
|
Thanks Amir1 & Thiemo_WMDE :), much much appreciated!
|
|
2019-01-30 17:01:41
|
<Amir1>
|
Anytime <3
|
|
2019-01-30 20:06:20
|
<McJill>
|
Is there a good reason why, on a page with only one revision, diff=prev loads just the one diff (wgDiffOldId is false, no .diff-otitle) while doing diff=cur or diff=next loads the two-column diff as if there were two revisions (wgDiffOldId and wgDiffNewId both true, etc.)?
|
|
2019-01-30 20:08:55
|
<McJill>
|
Whoops, sorry, probably better in #mediawiki
|
|
2019-01-30 22:20:14
|
<bawolff_>
|
xSavitar: I have no idea why phan seccheck is not repeatable on https://gerrit.wikimedia.org/r/#/c/mediawiki/extensions/Renameuser/+/487134/ :(
|
|
2019-01-30 22:27:53
|
<bawolff>
|
*sigh* i need to work more on that tool
|