Re: [PATCH] Revert "graph.c: mark private file-scope symbols as static"

2 messages, 2 authors, 2016-06-15 · open the first message on its own page

Re: [PATCH] Revert "graph.c: mark private file-scope symbols as static"

From: Thomas Rast <hidden>
Date: 2016-06-15 22:56:17

John Keeping [off-list ref] 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 <redacted>
---

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?

-- 
Thomas Rast
trast@{inf,student}.ethz.ch

Re: [PATCH] Revert "graph.c: mark private file-scope symbols as static"

From: John Keeping <hidden>
Date: 2016-06-15 22:56:17

On Sat, Mar 02, 2013 at 08:16:13PM +0100, Thomas Rast wrote:
John Keeping [off-list ref] writes:
quoted
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 <redacted>
---

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
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help