Code review comment for lp:~mvo/pkgme-devportal/i386-only-for-now

Revision history for this message
James Westby (james-w) wrote :

Hi Michael,

I'm not really sure what "supported" architectures mean here, could you help me
out?

37 + # XXX: mvo: it seems we don't need "all" here as we are interessted
38 + # in binary symbols only?

I'm not sure what that means, but it sounds like something we should have an
answer for before we land this?

53 +ARCHITECTURES = load_configuration().options.architectures_supported

Setting a module-level variable based on reading a config file is bad form,
because it will read the config file at import, which means it's hard
to write tests for example. Querying the config at use would be better.

Thanks,

James

review: Needs Information

« Back to merge proposal