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
