Skip to main content

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] [List Home]
Re: [mylyn-reviews-dev] Model in gerrit connector bundles

I fully agree that we should - if it makes sense - share coding, if we share functionality.

And I believe (although I still didn't have enough time to know :-() we also have some common data structures. I have just seen that the "complete" EMF model does not optimally reflect the Git & Gerrit domains - maybe I just do not understand. E.g. what is a "Topic", a "ReviewGroup" or a "Location"?

I think such abstractions should in general emerge, i.e. we see some commonality in the different review implementations and only then abstract over them.

I would like to see more modularity, i.e. in my opinion an editor that provides inline commenting should not depend on a model that would even allow navigating to other review items. (Don't get me wrong, it might make a lot of sense to allow a user to directly navigate through all diffs forming a Gerrit patch set, or whatever terminology applies to the different review implementations. It should just not be part of - tightly coupled to - the part that decorates the text compare editor with inline commenting capabilities.

Every review implementation could then decide on a case by case basis what framework components should be used and where it does not fit and an own implementation should be created.

I should start to get some time looking at the actual code and doing some real contributions. I noticed that I was already demoted to inactive committer status for not contributing a single commit ;-)

One more thing on EMF, we should think about only checking in the ecore (and genmodel?) files and generate during build time.

Cheers,
Jan

> -----Original Message-----
> From: mylyn-reviews-dev-bounces@xxxxxxxxxxx [mailto:mylyn-reviews-dev-
> bounces@xxxxxxxxxxx] On Behalf Of Steffen Pingel
> Sent: Freitag, 28. Januar 2011 09:51
> To: Mylyn Reviews Project
> Subject: Re: [mylyn-reviews-dev] Model in gerrit connector bundles
> 
> Sorry for the slow response. I have finally gotten around to capturing
> some of the discussion that happened in Vancouver on the wiki:
> http://wiki.eclipse.org/Reviews/Design .
> 
> I also took the feature matrix that Alvaro originally created an put
> it on the wiki: http://wiki.eclipse.org/Reviews/Features . It compares
> the reference implementation and shows some of the commonalities and
> differences.
> 
> Based on my experience with implementing an Eclipse integration for
> Atlassian's Crucible the most difficult part will be the SCM
> integration and the UI integration with different types of editors for
> inline commenting.
> 
> The SCM integration is easy for Gerrit since it's tightly coupled to
> Git and JGit/EGit already provide a good abstraction. While there
> could be long term advantages in using some of the Mylyn Versions API
> and UI integration I would not worry about that for now at all and
> directly couple to EGit.
> 
> For everything else, such as management of reviews, editor
> integration, annotation models, marker models, UI for commenting I see
> a large overlap between all reference implementations. While it is
> possible to limit abstraction to UI components I see a great benefit
> in also sharing a data model. This will reduce code duplication
> significantly, provide a powerful generic API for reviews and enable a
> richer feature set through focusing more collaborative efforts on the
> framework rather than specific implementations.
> 
> My impression is that the Gerrit data model is tailored towards usage
> with browsers and json and won't provide a great abstraction for
> Eclipse integration anyways and it is unclear how stable it will be in
> terms of API. While it involves some effort to map back and forth
> between a generic (EMF) model and Gerrit data model I am convinced
> that the long benefits outweigh the additional cost by far and will
> provide a more consistent and streamlined user experience.
> 
> Thoughts?
> 
> Steffen
> 
> PS: Please feel encouraged to update the wiki pages :).
> 
> 
> On Fri, Jan 21, 2011 at 2:24 PM, Lohre, Jan <jan.lohre@xxxxxxx> wrote:
> > I agree that TaskData will not be optimal for all data of the gerrit
> connector.
> >
> > But I think we should not start with a "framework" model at the
> moment. We will have to see what the different review systems have in
> common and then abstract over that.
> > E.g. (and this is really just an arbitrary example that got my
> immediate attention) FileRevision does not really make sense for git
> based reviews.
> >
> > Not that there is nothing to share, but for the moment the
> commonalities are IMHO sparse.
> >
> > On the other hand I still have to get into the existing code base.
> >
> > Cheers,
> > Jan
> > -----Original Message-----
> > From: mylyn-reviews-dev-bounces@xxxxxxxxxxx [mailto:mylyn-reviews-
> dev-bounces@xxxxxxxxxxx] On Behalf Of Steffen Pingel
> > Sent: Freitag, 21. Januar 2011 21:51
> > To: Mylyn Reviews Project
> > Subject: Re: [mylyn-reviews-dev] Model in gerrit connector bundles
> >
> > I'll post summarize on to the bug later today what motivated the
> current design.
> >
> >  324327: Define a common model for reviews
> >  https://bugs.eclipse.org/bugs/show_bug.cgi?id=324327
> >
> > On a high level, my sense is that TaskData is not a good match to
> > store the Gerrit Connector or review content in general. I think we
> > are best of limiting its use to task list integration and basic task
> > editor integration. From my experience, EMF offers much more powerful
> > capabilities (e.g. compare, merge, notifiactions...) and encapsulates
> > common concerns in the framework such as persistence that you have to
> > deal with otherwise.
> >
> > Steffen
> >
> >
> > On Fri, Jan 21, 2011 at 11:22 AM, Lohre, Jan <jan.lohre@xxxxxxx>
> wrote:
> >> Just noticed the model reviews.ecore in the core bundle of the
> gerrit
> >> connector.
> >> I don't like the idea of using EMF in the gerrit connector very much
> - we
> >> that have to map between gerrit and Mylyn TaskData and between
> TaskData and
> >> the model, don't we? What do we gain?
> >> Furthermore a very similar model exists in reviews framework.
> >>
> >> What do you think?
> >>
> >> Cheers,
> >> Jan
> >>
> >>
> >> _______________________________________________
> >> mylyn-reviews-dev mailing list
> >> mylyn-reviews-dev@xxxxxxxxxxx
> >> https://dev.eclipse.org/mailman/listinfo/mylyn-reviews-dev
> >>
> >>
> >
> >
> >
> > --
> > Steffen Pingel
> > Committer, http://eclipse.org/mylyn
> > Senior Developer, http://tasktop.com
> > _______________________________________________
> > mylyn-reviews-dev mailing list
> > mylyn-reviews-dev@xxxxxxxxxxx
> > https://dev.eclipse.org/mailman/listinfo/mylyn-reviews-dev
> > _______________________________________________
> > mylyn-reviews-dev mailing list
> > mylyn-reviews-dev@xxxxxxxxxxx
> > https://dev.eclipse.org/mailman/listinfo/mylyn-reviews-dev
> >
> 
> 
> 
> --
> Steffen Pingel
> Committer, http://eclipse.org/mylyn
> Senior Developer, http://tasktop.com
> _______________________________________________
> mylyn-reviews-dev mailing list
> mylyn-reviews-dev@xxxxxxxxxxx
> https://dev.eclipse.org/mailman/listinfo/mylyn-reviews-dev


Back to the top