From: John Keeping <john@keeping.me.uk>
To: Thomas Rast <trast@student.ethz.ch>
Cc: git@vger.kernel.org, Junio C Hamano <gitster@pobox.com>,
Johan Herland <johan@herland.net>
Subject: Re: [PATCH] Revert "graph.c: mark private file-scope symbols as static"
Date: Sun, 3 Mar 2013 10:29:46 +0000 [thread overview]
Message-ID: <20130303102946.GH7738@serenity.lan> (raw)
In-Reply-To: <87haktwr2a.fsf@pctrast.inf.ethz.ch>
On Sat, Mar 02, 2013 at 08:16:13PM +0100, Thomas Rast wrote:
> John Keeping <john@keeping.me.uk> writes:
>
> > This reverts commit ba35480439d05b8f6cca50527072194fe3278bbb.
> >
> > CGit uses these symbols to output the correct HTML around graph
> > elements. Making these symbols private means that CGit cannot be
> > updated to use Git 1.8.0 or newer, so let's not do that.
> >
> > Signed-off-by: John Keeping <john@keeping.me.uk>
> > ---
> >
> > I realise that Git isn't a library so making the API useful for outside
> > projects isn't a priority, but making these two methods public makes
> > life a lot easier for CGit.
> >
> > Additionally, it seems that Johan added graph_set_column_colors
> > specifically so that CGit should use it - there's no value to having
> > that as a method just for its use in graph.c and he was the author of
> > CGit commit 268b34a (ui-log: Colorize commit graph, 2010-11-15).
>
> Perhaps you could add a comment in the source to prevent this from
> happening again?
That feels wrong to me; would we really want a list of "$OUTSIDE_PROJECT
uses this" against all methods? CGit is using Git's internal API and so
should be prepared for breakage and to do what is necessary to work
around it - it's just this one case where adding a 2 line function to
Git makes CGit's life a lot easier.
I would hope that having this message in the history should be enough to
prevent this changing in the future; and it means that the comment is
associated with a date so that someone can decide to check whether CGit
is still using this function in the distant future.
John
next prev parent reply other threads:[~2013-03-03 10:30 UTC|newest]
Thread overview: 14+ messages / expand[flat|nested] mbox.gz Atom feed top
2013-03-02 12:46 [PATCH] Revert "graph.c: mark private file-scope symbols as static" John Keeping
2013-03-02 14:54 ` Johan Herland
2013-03-02 19:16 ` Thomas Rast
2013-03-03 10:29 ` John Keeping [this message]
2013-03-03 21:08 ` Junio C Hamano
2013-03-03 21:42 ` John Keeping
2013-03-03 22:49 ` Junio C Hamano
2013-03-03 23:24 ` John Keeping
2013-03-03 23:32 ` Junio C Hamano
2013-03-04 0:03 ` [PATCH v2] " John Keeping
2013-03-04 0:12 ` Jason A. Donenfeld
2013-03-04 3:42 ` Junio C Hamano
2013-03-04 4:25 ` Jason A. Donenfeld
2013-03-04 0:52 ` Johan Herland
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
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20130303102946.GH7738@serenity.lan \
--to=john@keeping.me.uk \
--cc=git@vger.kernel.org \
--cc=gitster@pobox.com \
--cc=johan@herland.net \
--cc=trast@student.ethz.ch \
/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.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.