[libcamera-devel] [PATCH 4/6] qcam: assets: Provide initial icon set

[Kieran Bingham]

Hi Laurent,

On 06/02/2020 23:42, Laurent Pinchart wrote:
> Hi Kieran,
> 
> Thank you for the patch.
> 
> On Thu, Feb 06, 2020 at 03:05:02PM +0000, Kieran Bingham wrote:
>> Provide simple clean icons from https://feathericons.com/
>> (https://github.com/feathericons/feather)
>>
>> These are provided under the MIT license.
> 
> Could you add a copy of the license to licenses/ ?

Looks like you've handled that with your licences series.



> 
>>
>> Signed-off-by: Kieran Bingham <kieran.bingham at ideasonboard.com>
>> ---
>>  src/qcam/assets/feathericons/README.md        |   5 +
>>  src/qcam/assets/feathericons/feathericons.qrc | 288 ++++++++++++++++++
>>  src/qcam/assets/feathericons/*.svg            |     +
>>
>> diff --git a/src/qcam/assets/feathericons/README.md b/src/qcam/assets/feathericons/README.md
>> new file mode 100644
>> index 000000000000..ce7664f6bf16
>> --- /dev/null
>> +++ b/src/qcam/assets/feathericons/README.md
>> @@ -0,0 +1,5 @@
>> +Icons from https://feathericons.com/
>> +License: MIT

Perhaps I should set this as the SPDX, although that would then perhaps
represent the README.md file ...?

>> +
>> +Generate the QRC file with:
>> + rcc --project
>> diff --git a/src/qcam/assets/feathericons/activity.svg b/src/qcam/assets/feathericons/activity.svg
>> new file mode 100644
>> index 000000000000..669a57a772fa
>> --- /dev/null
>> +++ b/src/qcam/assets/feathericons/activity.svg
>> @@ -0,0 +1 @@
> 
> Should we add an SPDX tag here ?
> 
> <!-- SPDX-License-Identifier: MIT -->

I don't think we should modify the files just to put a licence statement
in them no. They are an external import.


> 
>> +<svg xmlns="http://www.w3.org/2000/svg" width="24" height="24" viewBox="0 0 24 24" fill="none" stroke="currentColor" stroke-width="2" stroke-linecap="round" stroke-linejoin="round" class="feather feather-activity"><polyline points="22 12 18 12 15 21 9 3 6 12 2 12"></polyline></svg>
>> \ No newline at end of file
>>
>> < Remaing icons chopped >
> 
> Reviewed-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>

What are your thoughts on adding the full set, vs adding only the icons
used in the series 'so far'.

Adding the full set makes it easier to add functionality later and be
able to identify what icons are available, but of course comes at the
expense of storing more icons in the our repository ...


> 

-- 
Regards
--
Kieran