unofficial mirror of guix-patches@gnu.org 
 help / color / mirror / code / Atom feed
From: Jesse Gibbons <jgibbons2357@gmail.com>
To: Christopher Baines <mail@cbaines.net>,
	Danny Milosavljevic <dannym@scratchpost.org>
Cc: 39275@debbugs.gnu.org
Subject: [bug#39275] [PATCH] gnu: vlc: add fluidsynth as input.
Date: Sun, 26 Jan 2020 17:56:13 -0700	[thread overview]
Message-ID: <999e82b162db35d396932eb2c1d5a20f68c00959.camel@gmail.com> (raw)
In-Reply-To: <87r1zmikj6.fsf@cbaines.net>

On Sun, 2020-01-26 at 10:23 +0000, Christopher Baines wrote:
> 	Error verifying signature: Failed to execute gpg.
> Danny Milosavljevic <dannym@scratchpost.org> writes:
> 
> > Hi,
> > 
> > I'd prefer to change the subject to "gnu: vlc: Add MIDI support.".
> > 
> > > It would be good to put something in the commit message about why
> > > this
> > > is happening, as it's not obvious to me at least.
> > 
> > I disagree in general because I've seen before what that leads to:
> > Half the source code (the comments) end up in the commit messages
> > instead.
> > Eventually (because of some merge or something) those get lost, or
> > (because of a new commit) those get hidden.
> > Now someone is going to overlook it.
> 
> I'm not suggesting documenting code in commit messages, just saying
> something about the motivation/intention with the change is useful.
> The
> message you suggest meets this criteria perfectly.
> 
> > So I'd put documentation into comments, except when it's ephemeral
> > ("news").
> > 
> > Also, in the special case of Guix the reason why we add an input
> > later is
> > "we forgot to add it earlier and it was optional to the package and
> > thus the
> > package built" most of the time.  So the commit message would
> > always be
> > "we forgot earlier".
> 
> So I would say that "we forgot earlier" isn't a motivation, or
> descriptive of intent (although maybe it meets the "why this is
> happening" I mentioned earlier, but that was maybe poorly
> explained)..
> 
> > In this case, vlc got MIDI support with vlc 0.9.0 (long ago) and
> > nobody
> > noticed.
> > 
> > We could have the subject say "Add MIDI support" because it
> > communicates more
> > clearly what this commit does for users.  And that's the news :)
> 
> Yeah, I think that's good. At least in the context I was looking at
> this, trying to review the change, knowing what the intent is allows
> me
> to check if that's been satisfied (trying to play some MIDI content
> with
> VLC before and after).
> 
> Chris

Sorry for not explaining the intention. Do you want me to send a new
patch mentioning the added MIDI support? Should I put that in the
commit title, or in a comment?

  reply	other threads:[~2020-01-27  0:57 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-01-25  1:19 [bug#39275] [PATCH] gnu: vlc: add fluidsynth as input Jesse Gibbons
2020-01-25  8:35 ` Christopher Baines
2020-01-25 11:30   ` Danny Milosavljevic
2020-01-26 10:23     ` Christopher Baines
2020-01-27  0:56       ` Jesse Gibbons [this message]
2020-01-27  8:09         ` Christopher Baines

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=999e82b162db35d396932eb2c1d5a20f68c00959.camel@gmail.com \
    --to=jgibbons2357@gmail.com \
    --cc=39275@debbugs.gnu.org \
    --cc=dannym@scratchpost.org \
    --cc=mail@cbaines.net \
    /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).