> On 2011-11-26 10:09:50, Thomas Broyer wrote: > > The only benefit of waveRefFromHistoryToken over decodeWaveRefFromPath is > > the null return value vs. checked exception; every time > > waveRefFromHistoryToken is used, there's a check for a null valuejust > > after, so I wonder if waveRefFromHistoryToken is really worth it. > > > > I can see the value of these two "history-oriented" methods on > > HistorySupport, but not quite in the wave.model.waveref. > > > > The only dependency from 'wave' to 'box' is in EditToolbar, where > > decodeWaveRefFromPath could be used (along with a local try/catch). There's > > no real need to move the history-oriented methods out of HistorySupport, > > only to break the dependency of EditToolbar on HistorySupport (and > > actually, it doesn't look like the need in EditToolbar is about "history", > > it's really about the "wave ref"). > > > > (the comments below only apply if we choose to keep the methods in > > GwtWaverefEncoder)
Yeah, I also wasn't sure if it worth to keep the methods or maybe just use directly the methods in GwtWaverefEncoder. Maybe move them into a new HistorySupportUtil class? - Yuri ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/2935/#review3516 ----------------------------------------------------------- On 2011-11-26 08:42:28, Yuri Zelikov wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/2935/ > ----------------------------------------------------------- > > (Updated 2011-11-26 08:42:28) > > > Review request for wave and Christian Ohler. > > > Summary > ------- > > Fixes https://issues.apache.org/jira/browse/WAVE-306 > > > Diffs > ----- > > src/org/waveprotocol/box/webclient/client/HistoryChangeListener.java > b756541 > src/org/waveprotocol/box/webclient/client/HistorySupport.java abfd4c1 > src/org/waveprotocol/box/webclient/client/WebClient.java 738d0c2 > src/org/waveprotocol/wave/client/wavepanel/impl/toolbar/EditToolbar.java > 3e6ff45 > src/org/waveprotocol/wave/util/escapers/GwtWaverefEncoder.java 8983614 > > Diff: https://reviews.apache.org/r/2935/diff > > > Testing > ------- > > > Thanks, > > Yuri > >
