Code review comment for lp:~mvo/click/add-qmake-cross-to-chroot

Revision history for this message
Colin Watson (cjwatson) wrote :

09:38 <cjwatson> mvo: it's probably OK for now; but I think only qt5-qmake-cross-armhf exists and not other arches, so that probably ought to be arch-specific
09:39 <mvo> cjwatson: ok, so adding a "if target_arch == " ? thats fine with me, I really dislike special case like this, but its all about trade-offs here :)
09:40 <cjwatson> mvo: is it possible to write code that would install whichever of qt5-qmake-cross-armhf and qt5-qmake-arm-linux-gnueabihf (probably drop the -cross in the latter case) exists?
09:41 <cjwatson> then we could clean up the package naming later
09:41 <mvo> cjwatson: I was considering having a "optional_build_packages" that would get installed if they exist but ignored otherwise
09:41 <mvo> cjwatson: and yeah, I can add code (of course) to do all this, but the time it takes to write this is probably the same to just clean it up :)
09:42 <cjwatson> mvo: well, optional is problematic in case something goes wrong, I think it would be better to require one or the other
09:42 <cjwatson> (I agree this is all trade-offs, I'm not stating veto kind of opinions here ...)

So general approval for direction but I'll leave it to your judgement as to when the trade-offs are right for landing this.

review: Approve

« Back to merge proposal