[buildd-tools-devel] [PATCH 2/3] General chroot container member

[Jan-Marek Glogowski]

On Sun, 29 Mar 2009, Roger Leigh wrote:

> On Sun, Mar 29, 2009 at 09:38:01PM +0200, Jan-Marek Glogowski wrote:
> > This drops all chroot specific setters, getters and members
> > for a general chroot container member.
>
> I don't know if you had a chance to read my second patch review
> mail yet, but I would really appreciate knowing why this change
> is needed.
>
> In the rest of the original patch set, I don't see a single
> user of the generalised get_container() in the code, or
> CHROOT_CONTAINER in the setup scripts.  As a result, I am not
> certain what the generalisation is actually /for/.
>
> If it dropped all of the chroot-specific variables in the
> setup scripts I might understand it (though maybe not necessarily
> agree!), but I don't see the reason.  I'm all for generalising
> things, but in this case I need convincing it's cleaner (and the
> get_chroot_strings in the following patch is not IMO cleaner or
> more efficient than the current implementation).

I didn't have seen your review when splitting the original patch. Just got
this when writing a response to the 2nd review.

Jan-Marek