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

Hello,
I am also of the opinion to find the common items first, as we may
deviate too far at a later point,
we are talking reviews and they should all have common parts e.g.
Reviews, comments, review items, locations (to line ranges), etc..
Review Groups could be optional for some tools, but could be there in
case they are needed at a later point.

/Alvaro



On Sat, Jan 29, 2011 at 7:00 PM, Steffen Pingel
<steffen.pingel@xxxxxxxxxxx> wrote:
>> 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"?
>
> A Topic represents a thread of comments and location links a comment
> thread to a review artifact, e.g. it's a line in a file. You can find
> a more detailed description of the model in the PDF files on bug
> 324327.
>
>> 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.
>
> Given that we have successfully built review tooling implementations
> in the past I am in favor of doing a bit of design upfront based on
> what we already know and starting with that rather than merging at a
> later point. We will probably not get it right the first time and will
> end up adjusting the model in the future based on what we learn but I
> think we already have a good start.
>
>> 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.
>
> I don't see that having a model that contains all content of a review
> would prevent reuse of UI components. I would expect that the inline
> commenting functionality only depend on the portion of the model that
> relevant to providing that particular functionality. It's up to us to
> define these boundaries.
>
>> 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 think we'll get more benefit of a framework that is more than a UI
> toolbox. Here is a concrete example: I want to be able to link a patch
> set of a Gerrit review back to the task that describes the
> requirements for the proposed change and to the Hudson build that was
> kicked of when the patch set was pushed. If the data models are common
> - at least on the particular framework level - that's easy to do.
>
> The overarching goal of the Mylyn (top-level) project is to create a
> platform for ALM tools consisting of APIs and frameworks[1]. This
> includes providing excellent reference implementations for Open-Source
> tools such as Gerrit. At the same time we want to create frameworks
> that enable tight integration of ALM components and provide a platform
> for others to build on.
>
>> One more thing on EMF, we should think about only checking in the ecore (and genmodel?) files and generate during build time.
>
> I'm in favor of having all sources in Git for now. It makes it more
> difficult for contributors if tools are required for building that are
> not part of the Eclipse SDK.
>
> Steffen
>
>
> [1] http://wiki.eclipse.org/Mylyn/Charter
> _______________________________________________
> mylyn-reviews-dev mailing list
> mylyn-reviews-dev@xxxxxxxxxxx
> https://dev.eclipse.org/mailman/listinfo/mylyn-reviews-dev
>


Back to the top