Sorry, was to early in the morning ^^. I misread the Code - the naming of the methods in the interface are fine, don't bother.
Best Matthias 2015-08-28 9:12 GMT+02:00 Matthias Bohnstedt <matthias.bohnst...@gmail.com>: > Hi Denis, > > first of all I really appreciate you efforts to move the SarosSession to > the core, as this (besides the Negotiations) are still a bottleneck in the > current HTML-UI development. > > Regarding you proposals, this sounds reasonable to me. I don't know if > Stefan just informed you about the backgrounds of the "hack", or if this is > a matter that have to be considered for your solution, as I don't know what > the purpose of the SarosProjectMapper is. Otherwise +1 > > Best regards > Matthias > > PS: Would be nice if you would do the "TODO:renaming" befor you make > changes to the ISharedProjectListener (If you not already done it) . There > are also some other 'odd' things in the interface like > > public void userFinishedProjectNegotiation(User user); > > /** > * Is fired when an user leaves the shared project. > > When this is fired as some user leaves the shared Project, why not naming > it "UserLeavesProjectNegotiation" ? "finishing a project" have > a different meaning than "leaving a project" in my humble opinion, but maybe > I splitting hairs here... > > 2015-08-27 5:06 GMT+02:00 Stefan Rossbach <srossb...@arcor.de>: > >> These few "hacky" lines were part of the SarosProjectMapper that had >> reference to the SharedProject class. >> >> On 26.08.2015 22:30, Denis Washington wrote: >> > Hi, >> > >> > After the move of ChangeColorManager to the core is merged, the only >> > remaining non-core dependency of SarosSession will be >> SharedResourcesManager. >> > This dependency seems to simply be a hack: SarosSession calls a >> projectAdded() >> > method of SharedResourcesManager in addProjectMapping(), and >> projectRemoved() >> > on removeProjectMapping(). (A comment says "// HACK", but doesn't go >> into any >> > details.) >> > >> > Glorifying this hack by making it an interface in order to move >> SarosSession >> > to the core feels dirty. How about adding projectAdded() and >> projectRemoved() >> > methods to ISessionListener (ISharedProjectListener in master) instead? >> > SharedResourcesManager could listen for these events and SarosSession >> wouldn't >> > have to know anything about SharedResourcesManager's existence anymore >> - it >> > would just have to fire the events instead. >> > >> > Any objections? >> > >> > Regards, >> > Denis >> > >> > >> ------------------------------------------------------------------------------ >> > _______________________________________________ >> > DPP-Devel mailing list >> > DPP-Devel@lists.sourceforge.net >> > https://lists.sourceforge.net/lists/listinfo/dpp-devel >> >> >> >> ------------------------------------------------------------------------------ >> _______________________________________________ >> DPP-Devel mailing list >> DPP-Devel@lists.sourceforge.net >> https://lists.sourceforge.net/lists/listinfo/dpp-devel >> > >
------------------------------------------------------------------------------
_______________________________________________ DPP-Devel mailing list DPP-Devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/dpp-devel
