929909 - Rename Context::get to getSync

Status: RESOLVED FIXED

(L20n :: JS Library, defect, P1)

Description

[Staś Małolepszy :stas]

Let's recognize the fact that most of our methods are asynchronous by renaming get and getEntity to getSync and getEntitySync respectively.

We're consistently moving into a more asynchronous and non-blocking world, with all kinds of workers, services, langpacks etc.  Synchronous gets should be rare exceptions:  they make it harder or impossible to lazily fallback onto other languages.

We should advocate the use of Context::localize instead, and consider adding asynchronous get and getEntity in 1.1.

Comment 1

[Staś Małolepszy :stas]

Attachment: getSync and getEntitySync

The patch updates the tests, too.

I didn't rename getMany and getFromLocale, as they are internal, and they could be made to work asynchronously.  I filed bug 929918 for this.

Comment 2

[Zibi Braniecki [:zbraniecki][:gandalf]]

Comment on attachment 820881 [details] [diff] [review]
getSync and getEntitySync

Review of attachment 820881 [details] [diff] [review]:
-----------------------------------------------------------------

Hmm, I must say that I'm reluctant to get this without async get first... Just because it creates a void - there's no get, there's only getSync and a promise that soon we'll have async get for you...
I'm also worried about making such change in 1.0

Can you file a bug to design async get API? I'm not sure how should it look like.

Comment 3

[Staś Małolepszy :stas]

(In reply to Zbigniew Braniecki [:gandalf] from comment #2)

> Hmm, I must say that I'm reluctant to get this without async get first...
> Just because it creates a void - there's no get, there's only getSync and a
> promise that soon we'll have async get for you...
> I'm also worried about making such change in 1.0

I don't think this is a promise to do anything.  It just says what it does:  it gets the translation synchronously.  There may never even be an async counterpart, and that's fine.  By leaving the "get" name unassigned, we give ourselves the option to introduce it later on, or not.  If, OTOH, we leave "get" as it is right now, it will be very very hard to change it afterwards, and due to concerns about backwards compatibility, we'll likely end up with get (synchronous) and getAsync.

My point is that with getSync, we can add getAsync later on and choose to which of the two to alias "get".  If we keep "get" right now, we're stuck with it and there's no choosing in the future.

In fact, I don't want to add the async get right now.  We have Context::localize for that and it's enough.

From the educational point of view, I think that having getSync and localize is more likely to make people understand that they want to use localize.


> Can you file a bug to design async get API? I'm not sure how should it look
> like.

Bug 931707.

Comment 4

[Staś Małolepszy :stas]

Gandalf, we need a decision on this and I realized you didn't r+ nor r- my patch (btw it needs documentation fixes as well).  What is your take?

As I said before, this is about keeping our options open.  If we decide we don't need an async get, we can alias getSync under get with ease.  However, if we don't fix this bug, there's no going back:  get will be the sync one.

Comment 5

[Zibi Braniecki [:zbraniecki][:gandalf]]

yeah, I do agree, my only question I believe is wherever for 1.0 we want to alias getSync to get (to preserve the ctx.get notion) or do we want to evangelize people on getSync (I imagine having to explain that to Gaia devs).

I'll try to give you an answer over the next two days.

Comment 6

[Zibi Braniecki [:zbraniecki][:gandalf]]

Comment on attachment 820881 [details] [diff] [review]
getSync and getEntitySync

Review of attachment 820881 [details] [diff] [review]:
-----------------------------------------------------------------

and r+ for the patch, but don't land yet pls

Comment 7

[Staś Małolepszy :stas]

As agreed on the weekly call, landed: https://github.com/l20n/l20n.js/commit/ebeac2c899e573b113cafa73937f9e77f44b4f55