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

[Kieran Bingham]

On 13/02/2020 22:41, Laurent Pinchart wrote:
> Hi Kieran,
> 
> On Thu, Feb 13, 2020 at 10:38:03PM +0000, Kieran Bingham wrote:
>> On 06/02/2020 23:42, Laurent Pinchart wrote:
>>> 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 ...?
> 
> It would represent the README.md only, yes :-S
> 
>>>> +
>>>> +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.
> 
> I agree.
> 
> The REUSE specification includes a way to specify licenses for files
> that can't be modified, maybe we can use that.
> 
>>>> +<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 ...
> 
> It won't use much disk space, the icons are very small, but adding only
> the icons we need has the nice advantage of quickly showing which files
> are used (all of them). It's a bit like commented-out code, we usually
> strip it. I think I'd be in favour of only adding the icons we use, but
> it maybe not be difficult to convince me otherwise.


I agree in general, but in this instance, it's more about ensuring a
developer can easily extend qcam, and identify what icons they have to
work with from an existing design set.

I'm tempted to keep the icons in the assets location, but only specify
icons that we /use/ in the QRC file (so that we're not storing all
unused icons in the qcam binary itself).

And even better in the future, get the qrc file generated from an array
in meson...

-- 
Regards
--
Kieran