Code review comment for lp:~christian-roeller/hipl/whitelisting

Revision history for this message
Diego Biurrun (diego-biurrun) wrote :

On Thu, Sep 15, 2011 at 01:43:18PM +0000, Christian Röller wrote:
> > On Wed, Sep 14, 2011 at 07:43:47PM +0000, Stefan Götz wrote:
> > > Review: Needs Fixing
> > >
> > > As for Diego's comments, I agree wholeheartedly but I'd let you get
> > > away with a 'keep unrelated changes out of your next branch/merge
> > > proposal'.
> >
> > All of this really should not be that hard with the help of the rebase,
> > merge and replay bzr commands or even with diff and patch. The more
> > practice one gets using these tools, the better :)
> >
> > In all the projects I worked, unrelated changes in a patch were an
> > automatic reject, no further questions asked...
>
> I totally agree to that Diego, it was no good idea to mix up here different things.
> I just did it because I have changed all comments for the whitelisting relevant functions
> and just wanted to take care of consistency in the whole file.
>
> You are right a little bit more practice with bazaar couldn't be bad :),
> because this is the first time I really use it. But is it really worthwhile this time
> to create an own branch for it?

Yes.

> I mean I would just change a few comment alignment issues in one file.
> But as I said in the beginning I agree to that in principle.
> So if it is wished, I will try to do it. Even though this could take a
> few days, because I have to do first a lot of other things this week.

It really should not take you long. You can poke me on IRC if you have
trouble.

Diego

« Back to merge proposal