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
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
Marcus
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,
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
>
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
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
> 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
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
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.
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
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,