incubator-wave-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Vicente J. Ruiz Jurado" <v...@ourproject.org>
Subject Re: Review Request 12682: Compress HistoryX files in webclient/client/ into a single file.
Date Thu, 18 Jul 2013 16:05:12 GMT


> On July 17, 2013, 5:22 p.m., Yuri Zelikov wrote:
> > I think Kune use alternative implementation of History and that's the reason.
> 
> Ali Lown wrote:
>     @Vicente: Can you confirm is this is indeed the reason. (And so, whether this patch
would cause problems?)
> 
> Vicente J. Ruiz Jurado wrote:
>     These changes of History come from this review:
>     https://reviews.apache.org/r/2913/
>     that allow to intercept the history changes in applications that integrates wave
client (like kune) and allow make other uses of history with different hashtags (not only
waverefs).
>     
>     I think that this is needed to mantain to allow wave client third party integrations.

>     
>     I'll propose similar changes in other parts of the Webclient (for instance to allow
alternatives to Window class, alerts/prompts, etc).
> 
> Ali Lown wrote:
>     Hmm. I can understand making it easier for other applications, and am happy to support
a single-level of indirection here for that purpose. (The HistorySupport class, which can
be changed as third-party implementations wish). I don't feel that the three-level of indirection
system used here (HistorySupport -> HistoryProvider -> HistoryProviderDefault), which
whilst might make it less effort for third-parties to add their changes before shipping (why
is this a concern to us here?), is worth the cognitive effort of remembering about these 3
distinct concepts. I would not be a fan of this pattern being added elsewhere to the codebase.
>     
>     [This code-base is meant to be a reference implementation of Wave in a system, for
third-parties to use to build their additions to. It is much easier for a new third-party
to develop against a small code-base they can quickly understand.]
>     
>     [If some third-party has such an improved history system, why are they not sending
it back upstream (If they don't want to/can't, I don't see why running rebase is so difficult),
rather than changing upstream to make it easy for them? :)]

We don't have an improved history system. We are simply using other additional #hashtags (#register
#sign-in etc) not related with wave, something very common if you are using GWT.

Also allow to implement easily clean-urls or tiny urls, allowing to get from #some-clear-url
the equivalent token aka waveref.

This was the purpose of our review r2913.

If I remember correctly I was using the Delegation Pattern
http://en.wikipedia.org/wiki/Delegation_pattern
and, well, without something like gin injection in the client, it's a pattern that helps to
maintain these kind of different implementations and uses.

HistorySupport is also used in EditorToolbar and it can be used in other places on third party
codes as we do without patching to much.


- Vicente J.


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/12682/#review23302
-----------------------------------------------------------


On July 17, 2013, 3:19 p.m., Ali Lown wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/12682/
> -----------------------------------------------------------
> 
> (Updated July 17, 2013, 3:19 p.m.)
> 
> 
> Review request for wave, Bruno Gonzalez, Vicente J. Ruiz Jurado, and Yuri Zelikov.
> 
> 
> Repository: wave-git
> 
> 
> Description
> -------
> 
> Whilst perusing the code for other things to delete, I happened to spot these files.
> At which point I wondered why we had three classes doing a single-line function call
(to GWT).
> 
> As such, I compressed it all into the HistorySupport class. (Which still seems excessive,
but I will leave it there since it is still easy to change the single implementation [presuming
that was the reasoning behind using three classes in the first place]).
> 
> 
> Diffs
> -----
> 
>   src/org/waveprotocol/box/webclient/client/HistoryProvider.java f207b83 
>   src/org/waveprotocol/box/webclient/client/HistoryProviderDefault.java a2dae4a 
>   src/org/waveprotocol/box/webclient/client/HistorySupport.java 93fe852 
>   src/org/waveprotocol/box/webclient/client/WebClient.java d3e3b49 
> 
> Diff: https://reviews.apache.org/r/12682/diff/
> 
> 
> Testing
> -------
> 
> Builds and passes test suite.
> The composition of all 7 of these 'related' (but independent) patches is verified to
still work as a wave server.
> 
> 
> Thanks,
> 
> Ali Lown
> 
>


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message