> 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 > >
