Code review comment for lp://qastaging/~dooferlad/linaro-image-tools/fetch_image_cli

Revision history for this message
James Tunnicliffe (dooferlad) wrote :

On 16 June 2011 14:31, Loïc Minier <email address hidden> wrote:
> On Thu, Jun 16, 2011, James Tunnicliffe wrote:
>> >> +    if config.args['platform'] == "snapshot":
>> >> +        config.args['release_or_snapshot'] = "snapshot"
>> >> +    else:
>> >> +        config.args['release_or_snapshot'] = "release"
>> >
>> >  Instead of release_or_snapshot, "archive"?  or "source"?
>>
>> Release points to official Linaro releases, as currently hosted on
>> releases.linaro.org and shapshot the daily builds on
>> snapshots.linaro.org.
>
>  Yup; I understood the reference, but in a way it seemed to hardcode
>  that there would only be these two sources and their names in the
>  variable name.  It seemed like we could possibly change this over time,
>  perhaps dropping one, or adding one like a ci.linaro.org download
>  archive and using "release_or_snapshot" seemed a bit heavily biased
>  towards exactly 2 sources of downloads.

Ah, I see what you mean. Currently it is an indicator of the route
that the user took through the UI, which needs to be emulated for the
CLI. It is a bit hacky and I have ended up using it to build a table
name sometimes for database lookups, which I am sure I shouldn't! I
will see if I can restrict it to just the UI code where its meaning
should be more clear and think about a better name and usage.

--
James Tunnicliffe

« Back to merge proposal