Gerrit Topic Review Functionallity

1,533 views
Skip to first unread message

Diego Celix

unread,
Aug 30, 2011, 7:00:27 PM8/30/11
to Repo and Gerrit Discussion
Hi,

Following the discussion in the thread
http://groups.google.com/group/repo-discuss/browse_thread/thread/f47c422179073a17/56f13762b915b273?lnk=gst&q=Topic+Reviws#56f13762b915b273
We finally can say that the first approach of the Topic Review
functionallity is ready to be reviewed, and commented in order to
improve it and meet the requirements to be accepted by the gerrit
project. We have pushed it into the gerrit instance and here is a
shortcut:

https://review.source.android.com/#/q/status:open+project:tools/gerrit+branch:master+topic:TopicReviews,n,z

There are a few commits involving many changes, but all of them must
be tested together, because they are many dependencies between each
other.

As I said this is our firs approach, and there are some tasks that are
still not implemented, for example it is needed to update the
"canSubmit" function for topics using the prolog logic instead of the
old one.
As other example, it is needed to implement the "isMergeable"
functionallity for topics.

Anyway, at this point the Topic Review functionallity is fully
functional, and that tasks can be done a little bit later.

We would be pleased to read your thoughs about it.

Regars,

Diego Celix

Martin Fick

unread,
Aug 30, 2011, 7:59:03 PM8/30/11
to repo-d...@googlegroups.com, Diego Celix
On Tuesday 30 August 2011 05:00:27 pm Diego Celix wrote:
> Hi,
>
> Following the discussion in the thread
> http://groups.google.com/group/repo-discuss/browse_thread
> /thread/f47c422179073a17/56f13762b915b273?lnk=gst&q=Topic
> +Reviws#56f13762b915b273 We finally can say that the
> first approach of the Topic Review functionallity is
> ready to be reviewed, and commented in order to improve
> it and meet the requirements to be accepted by the
> gerrit project. We have pushed it into the gerrit
> instance and here is a shortcut:
>
> https://review.source.android.com/#/q/status:open+project
> :tools/gerrit+branch:master+topic:TopicReviews,n,z

Cool.



> There are a few commits involving many changes, but all
> of them must be tested together, because they are many
> dependencies between each other.

Any way you could restructure your patch series as a series
of functional changes? As is, it still seems like one large
change which happens to be broken into smaller pieces based
on where in the gerrit code base the code is. So while I am
grateful as a reviewer for the effort to make the changes
smaller, unfortunately, it really isn't.

By looking at the commit messages, I can get a good idea
where these changes touch code, and a general idea of what
the long term intent of the series might be related to, but
I can't figure out what any of the changes are actually
supposed to achieve functionally. This makes it extremely
hard to review since it basically means I have to review
everything before I can really even begin to guess at the
intent of the changes. This first review will be wasted
because I cannot evaluate correctness if I don't know the
intent.

Unfortunately, I feel that I set a really bad example for
some Gerrit changes when I uploaded my saved-searches
changes, because I made some of the same mistakes. It is
worth noting, that those changes are still sitting there
basically unreviewed and unmerged! :( Now that I actually
perform reviews, I realize that I wouldn't want to review
them.

So, my suggestion is to take the smallest functional piece
that you think could be merged, and focus on that piece.
Structure you submission around that piece. If even that
piece is too big, perhaps it involves some refactoring, that
would be a good thing to then break out as its own patch and
make the functional change depend on it (I see that you
factored out AbstractEntity from Change, that could be one
example.

I hope that you don't get too discouraged with the task of
splitting/organizing things differently. I am finding this is
one of the greatest challenges for myself, how to split
things. Hopefully it won't be too bad.


> As I said this is our firs approach, and there are some
> tasks that are still not implemented, for example it is
> needed to update the "canSubmit" function for topics
> using the prolog logic instead of the old one.
> As other example, it is needed to implement the
> "isMergeable" functionallity for topics.

As mentioned above, what is implemented? Hopefully you can
put more on that in the commit messages.

-Martin

--
Employee of Qualcomm Innovation Center, Inc. which is a
member of Code Aurora Forum

Diego Celix

unread,
Sep 1, 2011, 5:00:22 PM9/1/11
to Repo and Gerrit Discussion
Hi Martin,

What you said is totally right. I will try to restructure the commits
differently, so they can become a series of functional changes, making
easier the reviewing work.

Unfortunately, it is now a "spare time" project for me, so the things
may take some time...

Anyway, it would be great if somebody in the list would be interested
in trying the new functionallity and write down some impressions.


Regards,

Diego Celix

Marcus D. Hanwell

unread,
Sep 1, 2011, 6:29:28 PM9/1/11
to Diego Celix, Repo and Gerrit Discussion
On Thu, Sep 1, 2011 at 5:00 PM, Diego Celix <diego...@gmail.com> wrote:
> Hi Martin,
>
> What you said is totally right. I will try to restructure the commits
> differently, so they can become a series of functional changes, making
> easier the reviewing work.
>
> Unfortunately, it is now a "spare time" project for me, so the things
> may take some time...
>
> Anyway, it would be great if somebody in the list would be interested
> in trying the new functionallity and write down some impressions.

It is a shame that this is reduced to a spare time project now. We are
certainly very interested in this functionality too, and so I will
take a look, and if it looks usable we may deploy it and test it with
our user base. I would like to see this integrated in Gerrit, and
think that there are quite a few people out there who could use this
functionality.

Thanks for sharing the patches, I would also be interested in any
other high level overview you may have (what is implemented/missing,
approaches applied, test deployments).

Marcus

petefoth

unread,
Sep 2, 2011, 3:15:49 AM9/2/11
to Repo and Gerrit Discussion
On 01/09/2011 23:29, Marcus D. Hanwell wrote:
<snip>
>
> It is a shame that this is reduced to a spare time project now. We are
> certainly very interested in this functionality too, and so I will
> take a look, and if it looks usable we may deploy it and test it with
> our user base. I would like to see this integrated in Gerrit, and
> think that there are quite a few people out there who could use this
> functionality.
>
> Thanks for sharing the patches, I would also be interested in any
> other high level overview you may have (what is implemented/missing,
> approaches applied, test deployments).
>
> Marcus
>

Hi Marcus

You can find more information about what was implemented at
https://github.com/petefoth/gerrit-topic-reviews/wiki

As stated there, the intention was to 'fix' "Issue 51: ability to work
with
patchsets as well as single changes"
http://code.google.com/p/gerrit/issues/detail?id=51

The functionality was delivered to our customer several weeks ago
(the delay in pushing the changes upstream was due to a delay in
us getting our Contributor Agreements in and processed) - and has
been in use for a while. As we haven't received any bug reports or
complaints, it's clearly perfect ;-)

Regards

Pete

--
Pete Fotheringham
Codethink Ltd
http://codethink.co.uk

Chris Harris

unread,
Sep 5, 2011, 11:43:58 AM9/5/11
to Repo and Gerrit Discussion
Hi Peter,

I work with Marcus and have been looking at the topic review
functionality. I had one question. Is there a view that allows all
open topics along with their status to be viewed? . At the moment you
have to click through to a topic from a particular change to see the
topic status. We feel this would be a useful feature and would like to
investigate adding it if necessary.

Regards,

Chris

On Sep 2, 3:15 am, petefoth <pete.fothering...@gmail.com> wrote:
> On 01/09/2011 23:29, Marcus D. Hanwell wrote:
> <snip>
>
>
>
> > It is a shame that this is reduced to a spare time project now. We are
> > certainly very interested in this functionality too, and so I will
> > take a look, and if it looks usable we may deploy it and test it with
> > our user base. I would like to see this integrated in Gerrit, and
> > think that there are quite a few people out there who could use this
> > functionality.
>
> > Thanks for sharing the patches, I would also be interested in any
> > other high level overview you may have (what is implemented/missing,
> > approaches applied, test deployments).
>
> > Marcus
>
> Hi Marcus
>
> You can find more information about what was implemented athttps://github.com/petefoth/gerrit-topic-reviews/wiki

Diego Celix

unread,
Sep 5, 2011, 7:58:34 PM9/5/11
to Repo and Gerrit Discussion
Hi Chris,

At the moment there is no functionality that allows you to do that. We
rely on the current list of changes to access the topics. If you want
to avoid going to a specific change, and then going to the topic from
there, you can click directly in the topic name from the change list.
That will send you to the topic page, avoiding that intermediate step.

You are right, that could be a very useful feature, but it hadn't been
implemented in this first approach. Another useful feature would be
the direct access to the topic pressing the 't' key from the change
list (that is also not implemented) in the same way that you can
access a topic from the change review screen pressing the 't' key.
I will keep that in mind to see if it can be added when the basic work
is reviewed.

Thanks for your comments.

Regards,

Diego Celix

Chris Harris

unread,
Sep 6, 2011, 9:14:41 AM9/6/11
to Repo and Gerrit Discussion
Hi Diego,

One possible approach to this would be to include topics in the
status:open view ( ChangeTable.java ). So rather than listing out each
individual change for a topic one entry would be added to this table
representing the topic. This entry could have an expansion cross next
to the id, which if clicked on would expand to reveal all the changes
in the topic. Topics are essentially a grouping mechanism for changes,
so I think this might be an intuitive approach without having to add a
separate view just for topics.

Regards,

Chris

On Sep 5, 7:58 pm, Diego Celix <diego.ce...@gmail.com> wrote:
> Hi Chris,
>
> At the moment there is no functionality that allows you to do that. We
> rely on the current list of changes to access the topics. If you want
> to avoid going to a specific change, and then going to thetopicfrom
> there, you can click directly in thetopicname from the change list.
> That will send you to thetopicpage, avoiding that intermediate step.
>
> You are right, that could be a very useful feature, but it hadn't been
> implemented in this first approach. Another useful feature would be
> the direct access to thetopicpressing the 't' key from the change
> list (that is also not implemented) in the same way that you can
> access atopicfrom the changereviewscreen pressing the 't' key.
> I will keep that in mind to see if it can be added when the basic work
> is reviewed.
>
> Thanks for your comments.
>
> Regards,
>
> Diego Celix
>
> On Sep 5, 5:43 pm, Chris Harris <chris.har...@kitware.com> wrote:
>
>
>
>
>
>
>
> > Hi Peter,
>
> > I work with Marcus and have been looking at thetopicreview
> > functionality. I had one question. Is there a view that allows all
> > open topics along with their status to be viewed? . At the moment you
> > have to click through to atopicfrom a particular change to see the
> >topicstatus. We feel this would be a useful feature and would like to

Chris Harris

unread,
Sep 6, 2011, 9:28:19 AM9/6/11
to Repo and Gerrit Discussion
This the sort of tree table view I was thinking of:

http://google-web-toolkit-incubator.googlecode.com/svn/branches/dflorey/tableAddons/demo/TreeTableDemo/TreeTableDemo.html

Chris

On Sep 6, 9:14 am, Chris Harris <chris.har...@kitware.com> wrote:
> Hi Diego,
>
> One possible approach to this would be to include topics in the
> status:open view ( ChangeTable.java ). So rather than listing out each
> individual change for atopicone entry would be added to this table
> representing thetopic. This entry could have an expansion cross next
> to the id, which if clicked on would expand to reveal all the changes
> in thetopic. Topics are essentially a grouping mechanism for changes,

Marcus D. Hanwell

unread,
Sep 6, 2011, 11:38:32 AM9/6/11
to Chris Harris, Repo and Gerrit Discussion
On Tue, Sep 6, 2011 at 9:14 AM, Chris Harris <chris....@kitware.com> wrote:
> Hi Diego,
>
> One possible approach to this would be to include topics in the
> status:open view ( ChangeTable.java ). So rather than listing out each
> individual change for a topic one entry would be added to this table
> representing the topic. This entry could have an expansion cross next
> to the id, which if clicked on would expand to reveal all the changes
> in the topic. Topics are essentially a grouping mechanism for changes,
> so I think this might be an intuitive approach without having to add a
> separate view just for topics.
>
That is the kind of view we sketched out when designing the high level
open topic view, otherwise a topic branch with 20 commits in it would
dominate the open changes view. Looking at the topic branch view that
is pretty much what we wanted to add in, although I don't see anywhere
but that page that exposes the topic level review results. We would
also want to trigger builds of an entire topic, and not each commit in
a topic, using a CDash build scheduler.

Marcus

Martin Fick

unread,
Sep 21, 2011, 8:26:01 PM9/21/11
to repo-d...@googlegroups.com, petefoth
On Friday, September 02, 2011 01:15:49 am petefoth wrote:
> On 01/09/2011 23:29, Marcus D. Hanwell wrote:
>
> As stated there, the intention was to 'fix' "Issue 51:
> ability to work with
> patchsets as well as single changes"
> http://code.google.com/p/gerrit/issues/detail?id=51

I would like to know how your approach deals with conflicts
between individual changes in a topic and the topic as a
whole? In other words, imagine topic FOO with changes
A, B & C in it.

. What happens when change A is reviewed and it gets a -2,
but the topic gets a +2? Can the topic be submitted and
merged at that point?

. What about changes B & C if they get +2s, can they still
be submitted independently of A, of FOO?


Now, imagine topic BAR has changes D, E & F which do not
have any interdependencies, and someone submits topic BAR:

. What order will D, E & F get merged in?

. What happens if D, E & F are unsubmitted, and the topic
has a +2 and a new change G is upload for review to BAR?
Does the +2 on BAR disappear?

. What happens to topic BAR after it is submitted, can
someone upload new changes to review for it? Is the topic
name now taken, frozen?


Thanks,

Diego Celix

unread,
Sep 22, 2011, 7:11:31 PM9/22/11
to Repo and Gerrit Discussion
Hi Chris, Marcus


I really like that approach, I think it is a smart way to group the
changes belonging to a Topic.
But firstable I will try to focus on getting the basic funcionallity
ready to be reviewed. When that is done, I may give this a try or we
can open an issue making a proposal of the changes to be implemented.

I'm not familiar with the integration of a CDash build scheduler with
gerrit, and how it works. Are the builds triggered when a new commit
is merged into the destination branch of a given git repository?


Regards,

Diego

On 6 sep, 17:38, "Marcus D. Hanwell" <marcus.hanw...@kitware.com>
wrote:
> On Tue, Sep 6, 2011 at 9:14 AM, Chris Harris <chris.har...@kitware.com> wrote:
> > Hi Diego,
>
> > One possible approach to this would be to include topics in the
> > status:open view ( ChangeTable.java ). So rather than listing out each
> > individual change for atopicone entry would be added to this table
> > representing thetopic. This entry could have an expansion cross next
> > to the id, which if clicked on would expand to reveal all the changes
> > in thetopic. Topics are essentially a grouping mechanism for changes,
> > so I think this might be an intuitive approach without having to add a
> > separate view just for topics.
>
> That is the kind of view we sketched out when designing the high level
> opentopicview, otherwise atopicbranch with 20 commits in it would
> dominate the open changes view. Looking at thetopicbranch view that
> is pretty much what we wanted to add in, although I don't see anywhere
> but that page that exposes thetopiclevel review results. We would
> also want to trigger builds of an entiretopic, and not each commit in
> atopic, using a CDash build scheduler.
>
> Marcus

Diego Celix

unread,
Sep 22, 2011, 7:47:29 PM9/22/11
to Repo and Gerrit Discussion
Hi Martin,


This is our approach to the conflicts you mention:

On 22 sep, 02:26, Martin Fick <mf...@codeaurora.org> wrote:
> I would like to know how your approach deals with conflicts
> between individual changes in atopicand thetopicas a
> whole?  In other words, imaginetopicFOO with changes
> A, B & C in it.  
>
>   . What happens when change A is reviewed and it gets a -2,
> but thetopicgets a +2?  Can thetopicbe submitted and
> merged at that point?

No, if there are at least one change with a -2, the entire Topic gets
blocked and cannot be submitted. That doesnt mean that each change
belonging to the Topic must be reviewed. A Topic can be submited even
if the changes appear as not reviewed or not completely reviewed, but
if a change gets a -2 then the Topic cannot be submited.
 
>   . What about changes B & C if they get +2s, can they still
> be submitted independently of A, of FOO?

No, when a change belongs to a Topic, then all the changes must be
submited/merged or abandoned at once, using the Topic functionallity
for the desired purpose.

> Now, imaginetopicBAR has changes D, E & F which do not
> have any interdependencies, and someone submitstopicBAR:
>
>   . What order will D, E & F get merged in?  

The merge order will preserve the original git tree order on
submission. If the contributor has D, E and F in this order, then they
will be merged this way into the destination branch.

>   . What happens if D, E & F are unsubmitted, and thetopic
> has a +2 and a new change G is upload for review to BAR?  
> Does the +2 on BAR disappear?

Yes. A Topic has a ChangeSet which is analogous to a PatchSet in a
Change. Every time a modification to the topic is pushed (for example,
submitting a new PatchSet to a single Change belonging to the Topic),
then a new ChangeSet is created and the approvals will be now refered
to the last ChangeSet, which is new, so the previous +2 will
dissapear.

>   . What happens totopicBAR after it is submitted, can
> someone upload new changes to review for it?  Is thetopic
> name now taken, frozen?

If the Topic is submitted and merged, then it is allowed to create a
new Topic with the same name.

But for example, if you try to push to an existent topic a completely
unrelated change, I mean:

__ B __ C
/
A --|
\ __ D __ E

Where A is the common merged point.
B and C are the commits belonging to the Topic
D and E are the new commits which are tried to be pushed to the same
topic

Then they will be rejected because that will imply that all the
commits of the previous change set need to be abandoned. In this case,
the contributor must abandon the Topic manually first, to be able to
push the commits D and E to a new topic with the same name.


Hope that helps.


Regards,

Diego

Edwin Kempin

unread,
Sep 23, 2011, 2:23:23 AM9/23/11
to Diego Celix, Repo and Gerrit Discussion
Hi Diego,

I also have a question regarding the topic review functionality.
With Gerrit as it is now I'm able to group changes in a topic that do
not depend on each other. Will this change when the topic review
functionality gets merged?

Thanks and best regards,
Edwin

2011/9/23 Diego Celix <diego...@gmail.com>:

> --
> To unsubscribe, email repo-discuss...@googlegroups.com
> More info at http://groups.google.com/group/repo-discuss?hl=en
>

Diego Celix

unread,
Sep 23, 2011, 4:02:49 AM9/23/11
to Repo and Gerrit Discussion
Hi Edwin,

We thought about introducing a per-project setting to enable the Topic
Review funcionallity. By default it is disabled, so the normal
behaviour will remain.

If you enable the Topic Review functionallity, then yes, this will
change. We thought the Topic Review functionallity for reviewing
related changes, that will depend on each other. This is needed, for
example, to know if you are trying to delete a change in a Topic.

If you have a ChangeSet with commits A - B - C
Then you may modify your local git tree if you realise that, for
example, the commit B is not needed, so you will push the new changes.
The way you realise in gerrit that commit B is deleted is becuse C
doesnt need it anymore and now it depends directly on A.
If the unrelated changes are allowed, then if you push a new and
unrelated commit D, you will not be able to know if commits A - B - C
where deleted or what happened to them.

Another reason is because we thougth the new Topic entity a bit
different to the current topics. Right now it doesnt matter if you
reject a change that belongs to a topic because you are working with
individual changes and you can continue reviewing them individually.
With the new functionallity, if a change is rejected, the Topic gets
blocked and it cannot be submited. Thats because the actions happens
"atomically", so we think that if you are pushing a group of changes
to be reviewed at once and to submited/merged at once, they must be
related.


Your question made me re-think about one of the Martin's questions

> >> Now, imaginetopicBAR has changes D, E & F which do not
> >> have any interdependencies, and someone submitstopicBAR:
>
> >>   . What order will D, E & F get merged in?
>
> > The merge order will preserve the original git tree order on
> > submission. If the contributor has D, E and F in this order, then they
> > will be merged this way into the destination branch.
>

This answer is not correct, because that situation will not happen
because the changes need to be related to be part of the Topic. I
didn't realise about that in that moment.


Regards,

Diego

Chris Harris

unread,
Sep 23, 2011, 8:19:36 AM9/23/11
to Repo and Gerrit Discussion
Hi Diego,

We have actually started looking at providing some basic views and
searching capabilities for topics based on the branch on GitHub
(https://github.com/petefoth/gerrit-topic-reviews.git, I assume this
is where development continues). Initially, we are grouping topics and
change separately, combining them into a single table proved to be
more complicated than first thought. However, I think there is a lot
of commonality between changes and topics so some refactoring could be
done to provide a unified view.
The CDash build scheduler listens to the event stream to trigger new
builds when a new changeset has been added. We have added an event for
the creation of a changeset. What is left todo before the
functionality is ready for review? I want to try to avoid code any
features you already have in the pipeline,

Regards,

Chris

Diego Celix

unread,
Sep 26, 2011, 9:03:30 AM9/26/11
to Repo and Gerrit Discussion
Hi Chris,


> We have actually started looking at providing some basic views and
> searching capabilities for topics based on the branch on GitHub
> (https://github.com/petefoth/gerrit-topic-reviews.git, I assume this
> is where development continues).

Actually I'm working on my personal git server. The main difference
with the github branch is that in github is not rebased against the
gerrit master branch and some minor changes related with the rebase
process.

> the creation of a changeset. What is left todo before the
> functionality is ready for review? I want to try to avoid code any
> features you already have in the pipeline,

It is difficult to say because it is now a spare time project for me.
Some weeks I can spend more time on it than others. At the moment I'm
only refactoring the commits to be grouped with logical changes as
much as possible. But I don't have in mind adding new functionallities
at least until it is ready for being reviewed, so my pipeline is
empty ;)


Regards,

Diego

Diego Celix

unread,
Nov 6, 2011, 7:13:45 PM11/6/11
to Repo and Gerrit Discussion
Hi,

Finally it seems that I gathered enough time to finish this task. I've
reordered the commits and grouped them in a series of functional
changes. I hope this way it can be easier to be reviewed.

I'm not really up to date with the situation of the official gerrit
instance, and I don't know if there are any official mirror or any
instructions to follow in order to submit the code to be reviewed. So
meanwhile, I've pushed the changes to github ( https://github.com/dcelix/tools_gerrit
into the topic_reviews branch ). This way, the people interested in
this functionallity can see the code and try it with the most up-to
date version of gerrit I have.


Regards,

Diego

Chris Harris

unread,
Jan 31, 2012, 7:49:22 PM1/31/12
to Diego Celix, repo-d...@googlegroups.com
Hi Diego,

Now the gerrit server is back up are you planning on rebasing your
changes and pushing them for review? We have been working with the
topic review functionality here at kitware and are keen to see it
upstreamed in some form, we have a bunch changes/fixes based on it.

Regards,

Chris

On Nov 6 2011, 7:13 pm, Diego Celix <diego.ce...@gmail.com> wrote:
> Hi,
>
> Finally it seems that I gathered enough time to finish this task. I've
> reordered the commits and grouped them in a series of functional
> changes. I hope this way it can be easier to be reviewed.
>
> I'm not really up to date with the situation of the official gerrit
> instance, and I don't know if there are any official mirror or any
> instructions to follow in order to submit the code to be reviewed. So

> meanwhile, I've pushed the changes to github (https://github.com/dcelix/tools_gerrit

Chengwei

unread,
Feb 15, 2012, 2:28:26 AM2/15/12
to Repo and Gerrit Discussion
Hi Chris,

On Feb 1, 8:49 am, Chris Harris <chris.har...@kitware.com> wrote:
> Hi Diego,
>
> Now the gerrit server is back up are you planning on rebasing your
Is the gerrit server you mentioned is review.source.android.com? I
can't find tools/gerrit there.

Thanks,
Chengwei

> changes and pushing them for review? We have been working with thetopicreview functionality here at kitware and are keen to see it

Edwin Kempin

unread,
Feb 15, 2012, 2:50:51 AM2/15/12
to Chengwei, Repo and Gerrit Discussion


2012/2/15 Chengwei <chengwei...@gmail.com>

Hi Chris,

On Feb 1, 8:49 am, Chris Harris <chris.har...@kitware.com> wrote:
> Hi Diego,
>
> Now the gerrit server is back up are you planning on rebasing your
Is the gerrit server you mentioned is review.source.android.com? I
can't find tools/gerrit there.
No, the new Gerrit server that host the Gerrit project is https://gerrit-review.googlesource.com/
Find all the details in [1].

[1] http://groups.google.com/group/repo-discuss/browse_thread/thread/0cbb14bd09f12787
 

Thanks,
Chengwei

> changes and pushing them for review? We have been working with thetopicreview functionality here at kitware and are keen to see it
> upstreamed in some form, we have a bunch changes/fixes based on it.
>
> Regards,
>
> Chris
>
> On Nov 6 2011, 7:13 pm, Diego Celix <diego.ce...@gmail.com> wrote:
>
>
>
>
>
>
>
> > Hi,
>
> > Finally it seems that I gathered enough time to finish this task. I've
> > reordered the commits and grouped them in a series of functional
> > changes. I hope this way it can be easier to be reviewed.
>
> > I'm not really up to date with the situation of the official gerrit
> > instance, and I don't know if there are any official mirror or any
> > instructions to follow in order to submit the code to be reviewed. So
> > meanwhile, I've pushed the changes to github (https://github.com/dcelix/tools_gerrit
> > into the topic_reviews branch ). This way, the people interested in
> > this functionallity can see the code and try it with the most up-to
> > date version of gerrit I have.
>
> > Regards,
>
> > Diego

Marcus D. Hanwell

unread,
Feb 16, 2012, 10:19:37 AM2/16/12
to Diego Celix, repo-d...@googlegroups.com
On Tue, Jan 31, 2012 at 7:49 PM, Chris Harris <chris....@kitware.com> wrote:
> On Nov 6 2011, 7:13 pm, Diego Celix <diego.ce...@gmail.com> wrote:
>> Finally it seems that I gathered enough time to finish this task. I've
>> reordered the commits and grouped them in a series of functional
>> changes. I hope this way it can be easier to be reviewed.
>>
>> I'm not really up to date with the situation of the official gerrit
>> instance, and I don't know if there are any official mirror or any
>> instructions to follow in order to submit the code to be reviewed. So
>> meanwhile, I've pushed the changes to github (https://github.com/dcelix/tools_gerrit
>> into the topic_reviews branch ). This way, the people interested in
>> this functionallity can see the code and try it with the most up-to
>> date version of gerrit I have.
>>
> Now the gerrit server is back up are you planning on rebasing your
> changes and pushing them for review? We have been working with the
> topic review functionality here at kitware and are keen to see it
> upstreamed in some form, we have a bunch changes/fixes based on it.

It has been a little over half a month, and we have a running server
testing out many of these features (along with several bug fixes and
extensions we added). We are eager to start getting these features
into Gerrit, Diego could you upload your topic for review at the new
Gerrit instance?

If this is not possible, we would appreciate input on how we might
proceed in getting these changes integrated. The Qt project is also
using many of these enhancements, and has a wish to see the features
added upstream as far as I know.

Thanks,

Marcus

Chengwei

unread,
Feb 19, 2012, 10:10:21 PM2/19/12
to Repo and Gerrit Discussion
Thanks Edwin,

I have another question and can't find answer in this group. :-(.

How can I get Gerrit commit-msg hook to generate Change-Id in git
commit msg?

AFAIK, the Gerrit server doesn't run with a ssh daemon.

--
Thanks,
Chengwei

On Feb 15, 3:50 pm, Edwin Kempin <edwin.kem...@gmail.com> wrote:
> 2012/2/15 Chengwei <chengwei.yang...@gmail.com>
>
> > Hi Chris,
>
> > On Feb 1, 8:49 am, Chris Harris <chris.har...@kitware.com> wrote:
> > > Hi Diego,
>
> > > Now the gerrit server is back up are you planning on rebasing your
> > Is the gerrit server you mentioned is review.source.android.com? I
> > can't find tools/gerrit there.
>
> No, the new Gerrit server that host the Gerrit project ishttps://gerrit-review.googlesource.com/<http://www.google.com/url?sa=D&q=https://gerrit-review.googlesource.c...>
> Find all the details in [1].
>
> [1]http://groups.google.com/group/repo-discuss/browse_thread/thread/0cbb...

Thomas Broyer

unread,
Feb 20, 2012, 5:25:23 AM2/20/12
to repo-d...@googlegroups.com


On Monday, February 20, 2012 4:10:21 AM UTC+1, Chengwei wrote:
Thanks Edwin,

I have another question and can't find answer in this group. :-(.

How can I get Gerrit commit-msg hook to generate Change-Id in git
commit msg?

AFAIK, the Gerrit server doesn't run with a ssh daemon.

Diego Celix

unread,
Feb 23, 2012, 5:42:24 PM2/23/12
to Repo and Gerrit Discussion
Hi Marcus, Chris

I didn't notice about your replies until now. I'm also very interested
in seeing this functionallity upstreamed, but I don't know how many
time I will be able to spend on it.

I will try to get my self up to date with gerrit and the new changes
since my last rebase and I will let you know my planning then.

I'm sorry for this late reply...


Regards,

Diego

On Feb 16, 4:19 pm, "Marcus D. Hanwell" <marcus.hanw...@kitware.com>
wrote:
> On Tue, Jan 31, 2012 at 7:49 PM, Chris Harris <chris.har...@kitware.com> wrote:
> > On Nov 6 2011, 7:13 pm, Diego Celix <diego.ce...@gmail.com> wrote:
> >> Finally it seems that I gathered enough time to finish this task. I've
> >> reordered the commits and grouped them in a series of functional
> >> changes. I hope this way it can be easier to be reviewed.
>
> >> I'm not really up to date with the situation of the officialgerrit
> >> instance, and I don't know if there are any official mirror or any
> >> instructions to follow in order to submit the code to be reviewed. So
> >> meanwhile, I've pushed the changes to github (https://github.com/dcelix/tools_gerrit
> >> into the topic_reviews branch ). This way, the people interested in
> >> this functionallity can see the code and try it with the most up-to
> >> date version ofgerritI have.
>
> > Now thegerritserver is back up are you planning on rebasing your
> > changes and pushing them forreview? We have been working with the
> >topicreviewfunctionality here at kitware and are keen to see it

Marcus D. Hanwell

unread,
Feb 23, 2012, 7:16:44 PM2/23/12
to Diego Celix, Repo and Gerrit Discussion
Hi Diego,

On Thu, Feb 23, 2012 at 10:42 PM, Diego Celix <diego...@gmail.com> wrote:
> Hi Marcus, Chris
>
> I didn't notice about your replies until now. I'm also very interested
> in seeing this functionallity upstreamed, but I don't know how many
> time I will be able to spend on it.

That is great to hear.


>
> I will try to get my self up to date with gerrit and the new changes
> since my last rebase and I will let you know my planning then.

We have rebased all of the changes from the topic you published, along
with our fixes, quite recently. I just got back from a business trip,
and Chris is out of town until next week. It would be great to ensure
he has pushed the latest changes, but take a look at,

https://github.com/cjh1/gerrit


>
> I'm sorry for this late reply...
>

No problem, we are eager to begin the process of getting this
functionality reviewed and merged. Our current Gerrit installation is
running with these changes (and more from us) and has been working
well over the last few months.

Thanks,

Marcus

Dan Kegel

unread,
Nov 15, 2013, 4:15:52 PM11/15/13
to repo-d...@googlegroups.com, Diego Celix
On Thursday, February 23, 2012 4:16:44 PM UTC-8, Marcus D. Hanwell wrote:

On Thu, Feb 23, 2012 at 10:42 PM, Diego Celix <diego...@gmail.com> wrote:
> I didn't notice about your replies until now. I'm also very interested
> in seeing this functionallity upstreamed, but I don't know how many
> time I will be able to spend on it.

That is great to hear. ... We have rebased all of the changes from the topic you published, along

with our fixes,... take a look at,

https://github.com/cjh1/gerrit

I'm trying this out now.  Haven't managed to trigger the new behavior yet -
although I see the topics dashboard, approving all the changes in a topic
doesn't seem to cause the topic to appear on the topic dashboard for
approval.  I must be doing something wrong.

Is anybody other than Kitware using this branch?
- Dan

Chris Harris

unread,
Nov 18, 2013, 8:38:06 AM11/18/13
to Dan Kegel, Repo and Gerrit Discussion, Diego Celix
I think the Qt project is also using topic view, but not this branch. 

Marcus D. Hanwell

unread,
Nov 19, 2013, 9:47:45 PM11/19/13
to Chris Harris, Dan Kegel, Repo and Gerrit Discussion, Diego Celix
Last I heard they disabled topic review due to a number of bugs they
hit (several of which we fixed in the branch Chris pointed out). I
don't think the dashboard worked well for topics, but the topic view
is quite functional and entire topics can be approved/merged from the
topic view.

If a project has topic review enabled then individual commits can no
longer be merged, but clicking on the topic should take you to the
topic view page where the entire topic can be reviewed and/or merged.
We also trigger builds from the event stream when topics are updated,
and add comments pointing to the filtered CDash results.

Please let us know if you need any more information.

Marcus
Reply all
Reply to author
Forward
0 new messages