diff mbox

[1/1] font-awesome:new package

Message ID 1455601427-20322-1-git-send-email-atul.singh.mandla@rockwellcollins.com
State Changes Requested
Headers show

Commit Message

Atul Singh Feb. 16, 2016, 5:43 a.m. UTC
Font Awesome is a full suite of 605 pictographic icons for easy
 scalable vector graphics on websites.

Signed-off-by: Atul Singh <atul.singh.mandla@rockwellcollins.com>
---
 package/Config.in                      |  1 +
 package/font-awesome/Config.in         |  7 +++++++
 package/font-awesome/font-awesome.hash |  2 ++
 package/font-awesome/font-awesome.mk   | 23 +++++++++++++++++++++++
 4 files changed, 33 insertions(+)
 create mode 100644 package/font-awesome/Config.in
 create mode 100644 package/font-awesome/font-awesome.hash
 create mode 100644 package/font-awesome/font-awesome.mk

Comments

Thomas Petazzoni Feb. 16, 2016, 9:03 p.m. UTC | #1
Hello Atul,

Thanks for this contribution! Looks mostly good, I have a few comments
though, see below.

On Tue, 16 Feb 2016 11:13:47 +0530, Atul Singh wrote:
>  Font Awesome is a full suite of 605 pictographic icons for easy
>  scalable vector graphics on websites.

Initial space at the beginning of each line is not needed.

> diff --git a/package/Config.in b/package/Config.in
> index a5b31aa..a2bd46e 100644
> --- a/package/Config.in
> +++ b/package/Config.in
> @@ -854,6 +854,7 @@ menu "Graphics"
>  	source "package/cairomm/Config.in"
>  	source "package/exiv2/Config.in"
>  	source "package/fltk/Config.in"
> +	source "package/font-awesome/Config.in"
>  	source "package/fontconfig/Config.in"
>  	source "package/freetype/Config.in"
>  	source "package/gd/Config.in"

This package should rather go in the "Fonts, cursors, icons, sounds and
themes" menu, sub-menu "Icons".

> diff --git a/package/font-awesome/font-awesome.mk b/package/font-awesome/font-awesome.mk
> new file mode 100644
> index 0000000..acaa3d8
> --- /dev/null
> +++ b/package/font-awesome/font-awesome.mk
> @@ -0,0 +1,23 @@
> +################################################################################
> +#
> +# font-awesome
> +#
> +################################################################################
> +
> +FONT_AWESOME_VERSION = v4.5.0
> +FONT_AWESOME_SITE = $(call github,FortAwesome,Font-Awesome,$(FONT_AWESOME_VERSION))
> +FONT_AWESOME_LICENSE = OFLv1.1, MIT

License would be more precise with:

	OFLv1.1 (font), MIT (CSS, LESS and Sass files)

> +FONT_AWESOME_LICENSE_FILES = css/font-awesome.css

This is not a license text, it only repeats that it's under OFLv1.1
(font) and MIT (CSS). So I'd say, just don't specify any LICENSE_FILES.

> +FONT_AWESOME_DIRECTORIES_LIST = css fonts less scss

Out of curiosity, can you explain in which situation are the "less" and
"scss" directories useful ?

> +define FONT_AWESOME_INSTALL_TARGET_CMDS
> +	# Install required directories

Comment is not very useful.

> +	for dir in $(FONT_AWESOME_DIRECTORIES_LIST); \
> +	do \
> +		$(INSTALL) -d $(TARGET_DIR)/usr/share/font-awesome/$$dir && \
> +		$(INSTALL) -m 0644 -t $(TARGET_DIR)/usr/share/font-awesome/$$dir \
> +			$(@D)/$$dir/*.* || exit 1; \

We generally use "cp -dpfr" to copy full directories. What about simply:

	mkdir -p $(TARGET_DIR)/usr/share/font-awesome/
	$(foreach d,$(FONT_AWESOME_DIRECTORIES_LIST),\
		cp -dpfr $(@D)/$(d) $(TARGET_DIR)/usr/share/font-awesome$(sep))

Could you take into account those comments, and send an updated version?

Thanks a lot!

Thomas
Atul Singh Feb. 17, 2016, 7:12 a.m. UTC | #2
Hi Thomas,

>  Font Awesome is a full suite of 605 pictographic icons for easy
>  scalable vector graphics on websites.
*Initial space at the beginning of each line is not needed.*
ASM : I did not understand this exactly Since I have followed the usual
convention of a tab and 2 spaces. Could you please elaborate a bit more on
this.

> diff --git a/package/Config.in b/package/Config.in
> index a5b31aa..a2bd46e 100644
> --- a/package/Config.in
> +++ b/package/Config.in
> @@ -854,6 +854,7 @@ menu "Graphics"
>       source "package/cairomm/Config.in"
>       source "package/exiv2/Config.in"
>       source "package/fltk/Config.in"
> +     source "package/font-awesome/Config.in"
>       source "package/fontconfig/Config.in"
>       source "package/freetype/Config.in"
>       source "package/gd/Config.in"


*This package should rather go in the "Fonts, cursors, icons, sounds
andthemes" menu, sub-menu "Icons".*
ASM: I have added the package in the Fonts submenu under  "Fonts, cursors,
icons, sounds and
themes" menu, sub-menu "Icons".

> diff --git a/package/font-awesome/font-awesome.mk b/package/font-awesome/
font-awesome.mk
> new file mode 100644
> index 0000000..acaa3d8
> --- /dev/null
> +++ b/package/font-awesome/font-awesome.mk
> @@ -0,0 +1,23 @@
>
+################################################################################
> +#
> +# font-awesome
> +#
>
+################################################################################
> +
> +FONT_AWESOME_VERSION = v4.5.0
> +FONT_AWESOME_SITE = $(call github,FortAwesome,Font
-Awesome,$(FONT_AWESOME_VERSION))
> +FONT_AWESOME_LICENSE = OFLv1.1, MIT


*License would be more precise with:        OFLv1.1 (font), MIT (CSS, LESS
and Sass files)*
ASM: Updated the license information as suggested.

*> +FONT_AWESOME_LICENSE_FILES = css/font-awesome.css*


*This is not a license text, it only repeats that it's under OFLv1.1(font)
and MIT (CSS). So I'd say, just don't specify any LICENSE_FILES.*ASM:
Removed the FONT_AWESOME_LICENSE_FILES variable.


*> +FONT_AWESOME_DIRECTORIES_LIST = css fonts less scss*

*Out of curiosity, can you explain in which situation are the "less"
and"scss" directories useful ?*
ASM: As per my understanding less and scss are used for customization of
font awesome. There may be other reasons too.
source : *https://fortawesome.github.io/Font-Awesome/get-started/
<https://fortawesome.github.io/Font-Awesome/get-started/>*




*> +define FONT_AWESOME_INSTALL_TARGET_CMDS> +     # Install required
directories**Comment is not very useful.*
ASM: Removed the comment.








*> +     for dir in $(FONT_AWESOME_DIRECTORIES_LIST); \> +     do \> +
       $(INSTALL) -d $(TARGET_DIR)/usr/share/font-awesome/$$dir && \> +
         $(INSTALL) -m 0644 -t $(TARGET_DIR)/usr/share/font-awesome/$$dir
\> +                     $(@D)/$$dir/*.* || exit 1; \*



*We generally use "cp -dpfr" to copy full directories. What about simply:
      mkdir -p $(TARGET_DIR)/usr/share/font-awesome/        $(foreach
d,$(FONT_AWESOME_DIRECTORIES_LIST),\                cp -dpfr $(@D)/$(d)
$(TARGET_DIR)/usr/share/font-awesome$(sep))*
ASM: Updated the file as per your suggestion.


Best Regards,
Atul Singh.

On Wed, Feb 17, 2016 at 2:33 AM, Thomas Petazzoni <
thomas.petazzoni@free-electrons.com> wrote:

> Hello Atul,
>
> Thanks for this contribution! Looks mostly good, I have a few comments
> though, see below.
>
> On Tue, 16 Feb 2016 11:13:47 +0530, Atul Singh wrote:
> >  Font Awesome is a full suite of 605 pictographic icons for easy
> >  scalable vector graphics on websites.
>
> Initial space at the beginning of each line is not needed.
>
> > diff --git a/package/Config.in b/package/Config.in
> > index a5b31aa..a2bd46e 100644
> > --- a/package/Config.in
> > +++ b/package/Config.in
> > @@ -854,6 +854,7 @@ menu "Graphics"
> >       source "package/cairomm/Config.in"
> >       source "package/exiv2/Config.in"
> >       source "package/fltk/Config.in"
> > +     source "package/font-awesome/Config.in"
> >       source "package/fontconfig/Config.in"
> >       source "package/freetype/Config.in"
> >       source "package/gd/Config.in"
>
> This package should rather go in the "Fonts, cursors, icons, sounds and
> themes" menu, sub-menu "Icons".
>
> > diff --git a/package/font-awesome/font-awesome.mk
> b/package/font-awesome/font-awesome.mk
> > new file mode 100644
> > index 0000000..acaa3d8
> > --- /dev/null
> > +++ b/package/font-awesome/font-awesome.mk
> > @@ -0,0 +1,23 @@
> >
> +################################################################################
> > +#
> > +# font-awesome
> > +#
> >
> +################################################################################
> > +
> > +FONT_AWESOME_VERSION = v4.5.0
> > +FONT_AWESOME_SITE = $(call
> github,FortAwesome,Font-Awesome,$(FONT_AWESOME_VERSION))
> > +FONT_AWESOME_LICENSE = OFLv1.1, MIT
>
> License would be more precise with:
>
>         OFLv1.1 (font), MIT (CSS, LESS and Sass files)
>
> > +FONT_AWESOME_LICENSE_FILES = css/font-awesome.css
>
> This is not a license text, it only repeats that it's under OFLv1.1
> (font) and MIT (CSS). So I'd say, just don't specify any LICENSE_FILES.
>
> > +FONT_AWESOME_DIRECTORIES_LIST = css fonts less scss
>
> Out of curiosity, can you explain in which situation are the "less" and
> "scss" directories useful ?
>
> > +define FONT_AWESOME_INSTALL_TARGET_CMDS
> > +     # Install required directories
>
> Comment is not very useful.
>
> > +     for dir in $(FONT_AWESOME_DIRECTORIES_LIST); \
> > +     do \
> > +             $(INSTALL) -d $(TARGET_DIR)/usr/share/font-awesome/$$dir
> && \
> > +             $(INSTALL) -m 0644 -t
> $(TARGET_DIR)/usr/share/font-awesome/$$dir \
> > +                     $(@D)/$$dir/*.* || exit 1; \
>
> We generally use "cp -dpfr" to copy full directories. What about simply:
>
>         mkdir -p $(TARGET_DIR)/usr/share/font-awesome/
>         $(foreach d,$(FONT_AWESOME_DIRECTORIES_LIST),\
>                 cp -dpfr $(@D)/$(d)
> $(TARGET_DIR)/usr/share/font-awesome$(sep))
>
> Could you take into account those comments, and send an updated version?
>
> Thanks a lot!
>
> Thomas
> --
> Thomas Petazzoni, CTO, Free Electrons
> Embedded Linux, Kernel and Android engineering
> http://free-electrons.com
>
Thomas Petazzoni Feb. 17, 2016, 7:50 a.m. UTC | #3
Hello,

On Wed, 17 Feb 2016 12:42:51 +0530, Atul Singh Mandla wrote:

> >  Font Awesome is a full suite of 605 pictographic icons for easy
> >  scalable vector graphics on websites.
> *Initial space at the beginning of each line is not needed.*
> ASM : I did not understand this exactly Since I have followed the usual
> convention of a tab and 2 spaces. Could you please elaborate a bit more on
> this.

"One tab and 2 spaces" is the indentation needed for the help text of
the Config.in option.

For the commit log itself, no indentation shall be used.

Best regards,

Thomas
diff mbox

Patch

diff --git a/package/Config.in b/package/Config.in
index a5b31aa..a2bd46e 100644
--- a/package/Config.in
+++ b/package/Config.in
@@ -854,6 +854,7 @@  menu "Graphics"
 	source "package/cairomm/Config.in"
 	source "package/exiv2/Config.in"
 	source "package/fltk/Config.in"
+	source "package/font-awesome/Config.in"
 	source "package/fontconfig/Config.in"
 	source "package/freetype/Config.in"
 	source "package/gd/Config.in"
diff --git a/package/font-awesome/Config.in b/package/font-awesome/Config.in
new file mode 100644
index 0000000..065f554
--- /dev/null
+++ b/package/font-awesome/Config.in
@@ -0,0 +1,7 @@ 
+config BR2_PACKAGE_FONT_AWESOME
+	bool "font-awesome"
+	help
+	  Font Awesome is a full suite of 605 pictographic icons for easy
+	  scalable vector graphics on websites.
+
+	  https://github.com/FortAwesome/Font-Awesome
diff --git a/package/font-awesome/font-awesome.hash b/package/font-awesome/font-awesome.hash
new file mode 100644
index 0000000..d6b2be7
--- /dev/null
+++ b/package/font-awesome/font-awesome.hash
@@ -0,0 +1,2 @@ 
+# Locally Computed
+sha256  7813f416057da622b16229b10fef550e1dc64c5bb59871cd38fa86e76dfdbae8  font-awesome-v4.5.0.tar.gz
diff --git a/package/font-awesome/font-awesome.mk b/package/font-awesome/font-awesome.mk
new file mode 100644
index 0000000..acaa3d8
--- /dev/null
+++ b/package/font-awesome/font-awesome.mk
@@ -0,0 +1,23 @@ 
+################################################################################
+#
+# font-awesome
+#
+################################################################################
+
+FONT_AWESOME_VERSION = v4.5.0
+FONT_AWESOME_SITE = $(call github,FortAwesome,Font-Awesome,$(FONT_AWESOME_VERSION))
+FONT_AWESOME_LICENSE = OFLv1.1, MIT
+FONT_AWESOME_LICENSE_FILES = css/font-awesome.css
+FONT_AWESOME_DIRECTORIES_LIST = css fonts less scss
+
+define FONT_AWESOME_INSTALL_TARGET_CMDS
+	# Install required directories
+	for dir in $(FONT_AWESOME_DIRECTORIES_LIST); \
+	do \
+		$(INSTALL) -d $(TARGET_DIR)/usr/share/font-awesome/$$dir && \
+		$(INSTALL) -m 0644 -t $(TARGET_DIR)/usr/share/font-awesome/$$dir \
+			$(@D)/$$dir/*.* || exit 1; \
+	done
+endef
+
+$(eval $(generic-package))