| Message ID | 20200206150504.24204-5-kieran.bingham@ideasonboard.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series |
|
| Related | show |
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/ ? > > Signed-off-by: Kieran Bingham <kieran.bingham@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 > + > +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 --> > +<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@ideasonboard.com>
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@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@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 ... >
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@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@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.
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@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@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...
Hi Kieran, On Thu, Feb 13, 2020 at 10:44:39PM +0000, Kieran Bingham wrote: > On 13/02/2020 22:41, Laurent Pinchart wrote: > > 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@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@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). Works for me. > And even better in the future, get the qrc file generated from an array > in meson... If you have time to spare :-)
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 + +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 @@ +<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
Provide simple clean icons from https://feathericons.com/ (https://github.com/feathericons/feather) These are provided under the MIT license. Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com> --- src/qcam/assets/feathericons/README.md | 5 + src/qcam/assets/feathericons/feathericons.qrc | 288 ++++++++++++++++++ src/qcam/assets/feathericons/*.svg | + < Remaing icons chopped >