TitleValue

classic Classic list List threaded Threaded
11 messages Options
Reply | Threaded
Open this post in threaded view
|

TitleValue

Nikolas Everett
At the architecture summit yesterday we had a conversation about the TitleValue proposal and the vast majority of folks thought it was a great start. Something like 10% of us thought the patch _might_ be a start down the path to Javaify MediaWiki.  I was one of the 10%.

We resolved to talk more about the commit before merging it because of the objections of our minority. I felt somewhat vindicated. I slept on it. Now I don't think we made the right choice.  I think more discussion is a waste of time and we should just keep moving and try to catch the Javaification if it starts creeping in.  

The TitleValue proposal is an improvement over what we have now so I figure we should just do it.  Is that ok?

In the mean time, mostly to humor me, does anyone want to start a list on wiki of some of the Javaification things they are scared of?  I promise to contribute some paranoid ramblings which we can debate on the wiki or mailing list.

Thanks,

Nik
_______________________________________________
Wikitech-l mailing list
[hidden email]
https://lists.wikimedia.org/mailman/listinfo/wikitech-l
Reply | Threaded
Open this post in threaded view
|

Re: TitleValue

Siebrand Mazeland (WMF)
On Fri, Jan 24, 2014 at 11:19 AM, Nik Everett <[hidden email]>wrote:

> At the architecture summit yesterday we had a conversation about the
> TitleValue proposal and the vast majority of folks thought it was a great
> start. Something like 10% of us thought the patch _might_ be a start down
> the path to Javaify MediaWiki.  I was one of the 10%.
>
> We resolved to talk more about the commit before merging it because of the
> objections of our minority. I felt somewhat vindicated. I slept on it. Now
> I don't think we made the right choice.  I think more discussion is a waste
> of time and we should just keep moving and try to catch the Javaification
> if it starts creeping in.
>

Thank you for reconsidering, Nik. I fully support your proposal. The
concern obviously remains valid, and with all of our eyeballs on the code
and pre-merge review, we should be able to prevent disasters.

Cheers!

Siebrand
_______________________________________________
Wikitech-l mailing list
[hidden email]
https://lists.wikimedia.org/mailman/listinfo/wikitech-l
Reply | Threaded
Open this post in threaded view
|

Re: TitleValue

Brad Jorsch (Anomie)
In reply to this post by Nikolas Everett
On Fri, Jan 24, 2014 at 11:19 AM, Nik Everett <[hidden email]>wrote:

> At the architecture summit yesterday we had a conversation about the
> TitleValue proposal and the vast majority of folks thought it was a great
> start. Something like 10% of us thought the patch _might_ be a start down
> the path to Javaify MediaWiki.  I was one of the 10%.
>
> We resolved to talk more about the commit before merging it because of the
> objections of our minority. I felt somewhat vindicated. I slept on it. Now
> I don't think we made the right choice.  I think more discussion is a waste
> of time and we should just keep moving and try to catch the Javaification
> if it starts creeping in.
>

I'm disappointed, I thought you'd be our person to watch for the
Javaification.

It looks to me like the existing patch *already is* getting too far into
the Javaification, with it's proliferation of classes with single methods
that need to be created or passed around.


--
Brad Jorsch (Anomie)
Software Engineer
Wikimedia Foundation
_______________________________________________
Wikitech-l mailing list
[hidden email]
https://lists.wikimedia.org/mailman/listinfo/wikitech-l
Reply | Threaded
Open this post in threaded view
|

Re: TitleValue

Jeroen De Dauw-2
Hey,

Java is a technology with strong and weak sides, like any other.
Religiously labeling anything that resembles something from it as evil
because you do not like it is perhaps not the most constructive approach
one can take. That is quite obvious of course. From my vantage point, it
definitely seems like a lot of this is going on in the MediaWiki community
with regards to this topic and several others. Do you think this is not the
case - good for you. It is however the reason I am very reluctant to join
any such discussions, and I know this is the case for many other people as
well.

Cheers

--
Jeroen De Dauw
http://www.bn2vs.com
Don't panic. Don't be evil. ~=[,,_,,]:3
--
_______________________________________________
Wikitech-l mailing list
[hidden email]
https://lists.wikimedia.org/mailman/listinfo/wikitech-l
Reply | Threaded
Open this post in threaded view
|

Re: TitleValue

Tim Starling-2
In reply to this post by Nikolas Everett
On 24/01/14 11:19, Nik Everett wrote:
> The TitleValue proposal is an improvement over what we have now so I figure we should just do it.  Is that ok?

I am also personally in favour of it, but I would like to achieve
consensus of the MediaWiki core team if possible before continuing.

On 24/01/14 14:44, Brad Jorsch (Anomie) wrote:
> It looks to me like the existing patch *already is* getting too far into
> the Javaification, with it's proliferation of classes with single methods
> that need to be created or passed around.

The existing patch is somewhat misleading, since a TODO comment
indicates that some of the code in Linker should be moved to
HtmlPageLinkRenderer, instead of it just being a wrapper. So that
would make the class larger.

-- Tim Starling


_______________________________________________
Wikitech-l mailing list
[hidden email]
https://lists.wikimedia.org/mailman/listinfo/wikitech-l
Reply | Threaded
Open this post in threaded view
|

Re: TitleValue

Tim Starling-2
In reply to this post by Jeroen De Dauw-2
On 24/01/14 15:11, Jeroen De Dauw wrote:
> Java is a technology with strong and weak sides, like any other.
> Religiously labeling anything that resembles something from it as evil
> because you do not like it is perhaps not the most constructive approach
> one can take. That is quite obvious of course. From my vantage point, it
> definitely seems like a lot of this is going on in the MediaWiki community
> with regards to this topic and several others. Do you think this is not the
> case - good for you. It is however the reason I am very reluctant to join
> any such discussions, and I know this is the case for many other people as
> well.

I think the term "javaification" was introduced to the discussion by
Nik, who is the Java developer working on CirrusSearch, as a kind of
joking self-deprecation. I don't think anyone intended to imply that
everything to do with Java is bad. Daniel proposed an ideal code
architecture as consisting of a non-trivial network of trivial classes
-- a bold and precise vision. Nobody was uncivil or deprecating in
their response.

-- Tim Starling


_______________________________________________
Wikitech-l mailing list
[hidden email]
https://lists.wikimedia.org/mailman/listinfo/wikitech-l
Reply | Threaded
Open this post in threaded view
|

Re: TitleValue

Daniel Kinzler
In reply to this post by Brad Jorsch (Anomie)
Am 24.01.2014 16:15, schrieb Tim Starling:> On 24/01/14 15:11, Jeroen De Dauw wrote:
> Daniel proposed an ideal code
> architecture as consisting of a non-trivial network of trivial classes
> -- a bold and precise vision. Nobody was uncivil or deprecating in
> their response.

This idea is something that grew in my mind largely through discussions with
Jeroen, and the experience with the code he wrote for Wikidata. My gut feeling
for the right balance of locality vs. separation of concerns has shifted a lot
though that experience - in the beginning of the Wikidata project, I was a lot
more skeptical of the idea of "separation of concerns". Which doesn't mean I
insist of going down that road all the way all the time now.

I would like to take this opportunity to thank Jeroen for his conviction and the
work he has put into showing that DI and SoC actually make work with a big code
base less cumbersome. Without him, we would have copied more problems present in
the core code base.

One big advantage I want to highlight is confident refactoring: if you have
good, fine grained unit tests, it's easier to make large changes, because you
can be confident not to break anything.

Am 24.01.2014 14:44, schrieb Brad Jorsch (Anomie):
> It looks to me like the existing patch *already is* getting too far into
> the Javaification, with it's proliferation of classes with single methods
> that need to be created or passed around.

There is definitely room for discussion there. Should we have separate
interfaces for parsing and formatting, or should both be covered by the same
interface? Should we have a Linker interface for generating all kinds of links,
or separate interfaces (and/or implementations) for different kinds of links?

I don't have strong feelings about those, I'm happy to discuss the different
options. I'm not sure about the right place for that discussion though - the
patch? The RFC? This list?

Am 24.01.2014 15:56, schrieb Tim Starling:> On 24/01/14 11:19
> The existing patch is somewhat misleading, since a TODO comment
> indicates that some of the code in Linker should be moved to
> HtmlPageLinkRenderer, instead of it just being a wrapper. So that
> would make the class larger.

Indeed. HtmlPageLinkRenderer should take over much of the functionality
currently implemented by Linker. The implementation is going to be non-trivial.
I left that for later in order to keep the patch concise.

-- daniel





_______________________________________________
Wikitech-l mailing list
[hidden email]
https://lists.wikimedia.org/mailman/listinfo/wikitech-l
Reply | Threaded
Open this post in threaded view
|

Re: TitleValue

Nikolas Everett
On Fri, Jan 24, 2014 at 8:55 PM, Daniel Kinzler <[hidden email]>wrote:

> Am 24.01.2014 14:44, schrieb Brad Jorsch (Anomie):
> > It looks to me like the existing patch *already is* getting too far into
> > the Javaification, with it's proliferation of classes with single methods
> > that need to be created or passed around.
>
> There is definitely room for discussion there. Should we have separate
> interfaces for parsing and formatting, or should both be covered by the
> same
> interface? Should we have a Linker interface for generating all kinds of
> links,
> or separate interfaces (and/or implementations) for different kinds of
> links?
>
> I don't have strong feelings about those, I'm happy to discuss the
> different
> options. I'm not sure about the right place for that discussion though -
> the
> patch? The RFC? This list?
>
>
I vote mailing list.  Maybe it'll be livelier.

Personally, as I said in previous mails, I like the idea of pulling things
out of the Title class.

I'm going to pose questions and answer them in the order that they come to
me.

* Should linking, parsing, and formatting live outside the Title class?
Yes for a bunch of reasons.  At a minimum the Title class is just too large
to hold in your head properly.  Linking, parsing, and formatting aren't
really the worst offenders but they are reasonably easy to start with.  I
would, though, like to keep some canonical formatting in the new
TitleValue.  Just a useful __toString that doesn't do anything other than
print the contents in a form easy to read.

* Should linking, parsing, and formatting all live together in one class
outside the Title class?
I've seen parsing and formatting live together before just fine as they
really are the inverse of one another.  If they are both massively complex
then they probably ought not to live together.  Linking feels like a thing
that should consume the thing that does formatting.  I think putting them
together will start to mix metaphors too much.

* Should we have a formatter (or linker or parser) for wikitext and another
for html and others as we find new output formats?
I'm inclined against this both because it requires tons of tiny classes
that can make tracing through the code more difficult and because it
implies that each implementation is substitutable for the other at any
point when that isn't the case.  Replacing the html formatter used in the
linker with the wikitext formatter would produce unusable output.


I really think that the patch should start modifying the Title object to
use the the functionality that it is removing from it.  I'm not sure we're
ready to start deprecating methods in this patch though.


In a parallel to getting the consensus to merge a start on TitleValue we
need to be talking about what kind of inversion of control we're willing to
have.  You can't step too far down the services path without some kind of
strategy to prevent one service from having to know what its dependencies
dependencies are.

Nik
_______________________________________________
Wikitech-l mailing list
[hidden email]
https://lists.wikimedia.org/mailman/listinfo/wikitech-l
Reply | Threaded
Open this post in threaded view
|

Re: TitleValue

Daniel Kinzler
Thanks for your input Nik!

I'll add my 2ยข below. Would be great if others could chime in.

I have just pushed a new version of the path, please have a look at
https://gerrit.wikimedia.org/r/#/c/106517/

Am 04.02.2014 16:31, schrieb Nikolas Everett:
> * Should linking, parsing, and formatting live outside the Title class?
> Yes for a bunch of reasons.  At a minimum the Title class is just too large
> to hold in your head properly.  Linking, parsing, and formatting aren't
> really the worst offenders but they are reasonably easy to start with.

Indeed

>  I
> would, though, like to keep some canonical formatting in the new
> TitleValue.  Just a useful __toString that doesn't do anything other than
> print the contents in a form easy to read.

done

> * Should linking, parsing, and formatting all live together in one class
> outside the Title class?
> I've seen parsing and formatting live together before just fine as they
> really are the inverse of one another.  If they are both massively complex
> then they probably ought not to live together.

There are two questions here: should they be defined in the same interface? And
should they be implemented by the same class? Perhaps the answer is no to the
former, but yes to the latter...

A good argument for them sharing an implementation is the fact that both
formatting and parsing requires the same knowledge: Namespace names and aliases,
as well as normalization rules.

> Linking feels like a thing
> that should consume the thing that does formatting.  I think putting them
> together will start to mix metaphors too much.

Indeed

> * Should we have a formatter (or linker or parser) for wikitext and another
> for html and others as we find new output formats?
> I'm inclined against this both because it requires tons of tiny classes
> that can make tracing through the code more difficult

maybe, but I don't think so

> and because it
> implies that each implementation is substitutable for the other at any
> point when that isn't the case.  Replacing the html formatter used in the
> linker with the wikitext formatter would produce unusable output.

That's a compelling point, I'll try and fix this in the next iteration. Thanks!

> I really think that the patch should start modifying the Title object to
> use the the functionality that it is removing from it.  I'm not sure we're
> ready to start deprecating methods in this patch though.

I agree. I was reluctant to mess with Title just yet, but it makes sense to
showcase the migration path and remove redundant code.


> In a parallel to getting the consensus to merge a start on TitleValue we
> need to be talking about what kind of inversion of control we're willing to
> have.  You can't step too far down the services path without some kind of
> strategy to prevent one service from having to know what its dependencies
> dependencies are.

Let's try and be clear about how "inversion of control" relates to "dependency
injection": you can have IoC without CI (e.g. hooks/listeners, etc), and DI
without IoC (direct injection via constructor or setter). In fact, direct DI
without IoC is generally preferable, since it is more explicit and easier to
test. Specifically, passing in a "kitchen sink" registry object should be
avoided, since it makes it hard to know what collaborators a service *actually*
needs.

You need IoC only if the construction of a service we need must be deferred for
some reason. Prime reasons are

a) performance (lazy construction of part of the object graph)

b) information needed for the construction of the service is only known later
(this is really a code small, indicating a design issue - the "service" wasn't
really designed as a service).

In any case, yes, we'll need IoC for DI in some cases. In my experience, the
best approach usually turns out to be one of the following two:

1) provide a builder function. This is flexible and convenient. The downside is
that there is no type hinting/checking, you have to trust that the callback
actually implements the expected signature. A single-method factory interface
can fix that, but since PHP doesn't have inline classes, these are not as
convenient to use.

2) provide a registry that manages the creation and re-use of different
instances of a certain kind of sing, e.g. a SerializerRegistry managing
serializers for different kinds of things. We may not know in advance what kind
of thing we'll need to serialize, so we need to have the registry/factory
around. In the simple case, this could be handled via (1) by simply wrapping the
registry in a closure, but we may want to access some extra info from the
registry, e.g. which serializers are supported, etc.

I don't think we should pick one over the other, just make clear when to use
which approach. I can't think of a use case that isn't covered by one of the
two, though.

-- daniel

_______________________________________________
Wikitech-l mailing list
[hidden email]
https://lists.wikimedia.org/mailman/listinfo/wikitech-l
Reply | Threaded
Open this post in threaded view
|

Re: TitleValue

Sumana Harihareswara-2
I agree that this mailing list is a reasonable place to discuss the
interfaces.

Notes from the Architecture Summit are now up at
https://www.mediawiki.org/wiki/Architecture_Summit_2014/TitleValue# . At
yesterday's RFC review we agreed that we'd like to hold another one next
week (will figure out a good date/time with Nik, Daniel, and the
architects) and discuss TitleValue, see if there's anything that needs
moving forward.

Sumana Harihareswara
Engineering Community Manager
Wikimedia Foundation
_______________________________________________
Wikitech-l mailing list
[hidden email]
https://lists.wikimedia.org/mailman/listinfo/wikitech-l
Reply | Threaded
Open this post in threaded view
|

Re: TitleValue

Daniel Kinzler
Am 06.02.2014 21:09, schrieb Sumana Harihareswara:
> I agree that this mailing list is a reasonable place to discuss the
> interfaces.
>
> Notes from the Architecture Summit are now up at
> https://www.mediawiki.org/wiki/Architecture_Summit_2014/TitleValue# . At
> yesterday's RFC review we agreed that we'd like to hold another one next
> week (will figure out a good date/time with Nik, Daniel, and the
> architects) and discuss TitleValue, see if there's anything that needs
> moving forward.

That would be great, better still if it was during business-hours for me :)

I'm currently working on an alternative approach to the PageLinker and
TitleFormatter interfaces, which would result in fewer classes. I'm not sure
whether that approach is actually better yet, but since several people have
expressed a preference for this, I would like to give it a try. I hope to have
this done some time next week.

-- daniel


_______________________________________________
Wikitech-l mailing list
[hidden email]
https://lists.wikimedia.org/mailman/listinfo/wikitech-l