[hipl-dev] Re: [Merge] lp:~toxedvirus/hipl/hipfw-modules into lp:hipl
- From: Diego Biurrun <diego@xxxxxxxxxx>
- To: mp+40891@xxxxxxxxxxxxxxxxxx
- Date: Tue, 16 Nov 2010 15:37:22 +0100
On Mon, Nov 15, 2010 at 07:01:57PM +0000, Andrius Bentkus wrote:
> On Mon, Nov 15, 2010 at 7:37 PM, Diego Biurrun <diego@xxxxxxxxxx> wrote:
>
> > On Mon, Nov 15, 2010 at 05:44:33PM +0000, Andrius Bentkus wrote:
> > > On Mon, Nov 15, 2010 at 6:21 PM, Diego Biurrun <diego@xxxxxxxxxx> wrote:
> > >
> > > > Review: Needs Fixing
> > > > one-minute review: This contains unrelated bug fixes (multiple
> > > > inclusion guard names in the modules subdirectory) and there are
> > > > some K&R style nits to address.
> > >
> > > This contains unrelated bug fixes => I will take this next time
> > > into account, but I won't do it in this branch, this is just
> > > overhead I'm not willing to spend my time on. Just review it and
> > > merge.
> >
> > I have reviewed this branch and found issues with it. Getting
> > branches merged requires addressing reviewer comments. Code reviews
> > do wonders to software quality if done properly. Of course this
> > requires actually implementing the requested changes. If reviewer
> > feedback is simply ignored, the time spent on reviewing the code was
> > wasted.
>
> I have done that until now.
Yes. We have introduced a code review culture in HIPL inspired by my
experience from real-world and very successful FOSS projects and we are
quite happy with the overall results.
It is clear that you are not accustomed to code reviews and see them as
nothing more than an annoyance. I suggest that you see them as an
opportunity to improve your coding skills. I have learned a great deal
by having other people look over my code and the sentiment is shared
across HIPL since we started code reviews.
> > Committing separate changes separately is not overhead. On the contrary,
> > it reduces debugging time. Since in large projects debugging and
> > maintenance work dwarfs raw development time, this is a very useful
> > tradeoff to make.
>
> I agree with you, but there is no bazaar workflow defined in doc/HACKING
> therefore there is no possible way for me to know THAT until you inform me
> personally. If you want all the developers to comply to a workflow you have
> in mind, please add it to doc/HACKING, otherwise every new member which is
> not familiar will have to spend time guessing how you want stuff done.
I will, once we have all these workflows figured out. Right now they
are still being developed. We switched to Bazaar only recently and
nobody knows all the detailed ins and outs yet.
That said, the history of your branch has little value for mainline, so
I don't see why we need to strive to maintain it completely. That leaves
good old patches as a possibility. To cherrypick specific revisions from
your branch onto trunk, try 'bzr replay'. I used that to push changes
from my packaging branch upstream.
Diego
Other related posts:
