unofficial mirror of guix-devel@gnu.org 
 help / color / mirror / code / Atom feed
From: L  p R n  d n <guix@lprndn.info>
To: guix-devel@gnu.org
Subject: Re: First patch & hello
Date: Wed, 14 Nov 2018 16:51:24 +0100	[thread overview]
Message-ID: <10edc45bbfff91735097e6af9f214d9d@posteo.net> (raw)
In-Reply-To: <871s7nvp9a.fsf@elephly.net>

Ricardo Wurmus <rekado@elephly.net> writes:

> Hi,
> 
> thank you for your contribution!
> 
>> 1 . I put the package under fontutils.scm but maybe there's a better
>> place for a font viewer?
> 
> This seems fine to me.
> 
>> 2. I found there is a glib-or-gtk-build-system, I hesitated to use as
>> Im̀
>> not sure what its purpose. Can someone clarify its use?
> 
> The glib-or-gtk-build-system is an extension of the gnu-build-system
> that is useful for GNOME packages and other packages that require a
> little more setup after installation.  Here’s what the comments in
> “guix/build-system/glib-or-gtk.scm” say:
> 
> ;; This build system is an extension of the 'gnu-build-system'.  It
> ;; accomodates the needs of applications making use of glib or gtk+ 
> (with "or"
> ;; to be interpreted in the mathematical sense).  This is achieved by 
> adding
> ;; two phases run after the 'install' phase:
> ;;
> ;; 'glib-or-gtk-wrap' phase:
> ;;
> ;; a) This phase looks for GSettings schemas, GIO modules and theming 
> data.
> ;; If any of these is found in any input package, then all programs in
> ;; "out/bin" are wrapped in scripts defining the nedessary environment
> ;; variables.
> ;;
> ;; b) Looks for the existence of "libdir/gtk-3.0" directories in all 
> input
> ;; packages.  If any is found, then the environment variable "GTK_PATH" 
> is
> ;; suitably set and added to the wrappers.  The variable "GTK_PATH" has 
> been
> ;; preferred over "GTK_EXE_PREFIX" because the latter can only point to 
> a
> ;; single directory, while we may need to point to several ones.
> ;;
> ;; 'glib-or-gtk-compile-schemas' phase:
> ;;
> ;; Looks for the presence of "out/share/glib-2.0/schemas".  If that 
> directory
> ;; exists and does not include a file named "gschemas.compiled", then
> ;; "glib-compile-schemas" is run in that directory.
> 
> 
>> 3. I tried to get a clean patch but it needed some manual work. What
>> your workflow to produce patches? (I use emacs)
> 
> Usually you would make a local git commit and then run
> 
>     git format-patch -1
> 
> to generate a patch file from that commit.  You could also directly 
> send
> it to the mailing list with “git send-email”.
> 
> What follows are some comments about the patch.  I’m trying to be extra
> thorough; please don’t let this discourage you.  Some of my comments
> will just be matters of opinion :)

Thanks! It was exactly what I was looking for!

>> Subject: [PATCH] gnu: Add font-manager.
>> 
>> ---
> 
> It seems that the commit message is missing here.  In this case the
> complete commit message would be:
> 
> gnu: Add font-manager.
> 
> * gnu/packages/fontutils.scm (font-manager): New variable.
> 

Do you know why it wasn't added? I used magit format-patch.

> 
> Please remove the last sentence.  Please also reindent the description
> string (there should be no indentation for following lines).  Also
> please use two spaces after sentences.  “guix lint” will remind you of
> these things

Is there a way to specify a custom location for "guix lint"? A little
bit like the "-L" option from "guix build" for example?
I suppose the right way to invoke 'guix lint' should be to use 
"./pre-inst-env" in a local
guix repository, but I didn't find where I was supposed to find it. :/

> Could you please send an updated patch to guix-patches@gnu.org?
> 

I will soon. I just tested to build the package with "rounds=2" and it
seems the package isn't deterministic. Is it acceptable or a total
no-go?

Thanks again,

Lprndn

  reply	other threads:[~2018-11-14 15:51 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-11-13 17:35 First patch & hello L p R n d n
2018-11-14 12:04 ` Ricardo Wurmus
2018-11-14 15:51   ` L p R n d n [this message]
2018-11-14 19:07     ` swedebugia
2018-11-14 19:58     ` Ricardo Wurmus
2018-11-15 23:08       ` L p R n d n

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: https://guix.gnu.org/

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=10edc45bbfff91735097e6af9f214d9d@posteo.net \
    --to=guix@lprndn.info \
    --cc=guix-devel@gnu.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this public inbox

	https://git.savannah.gnu.org/cgit/guix.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).