Thread (2 messages) 2 messages, 2 authors, 2019-07-10

Re: [PATCH v3 07/14] apply: make parse_git_header public

From: Thomas Gummerer <hidden>
Date: 2019-07-10 08:48:58

On 07/09, Junio C Hamano wrote:
Thomas Gummerer [off-list ref] writes:
quoted
Maybe it would be even better to name it 'struct gitdiff_data', as
it's really only used for those few functions?
Is it really the case where "these three are only used by the
codepath you made public"?  If so, I agree that "gitdiff_data" is a
perfectly good name for it.

I however had an impression that it is the oppposite, i.e. "the
codepath you made public only needs these three, but these three are
used by other (still private) parts, too."  If this is the case,
then "gitdiff_data" is a misnomer, if we were to embed an instance
inside apply_state.
Yeah, that's correct.  What I meant was that since we're only using
this struct for the private 'gitdiff_*()' functions, which are called
from 'parse_git_diff_header()', 'struct gitdiff_data' would be a
better name than 'struct parse_git_diff_header_data'.

I do agree that it wouldn't be a good name if we were to embed it
inside 'struct apply_state', and as mentioned in the previous email
I'd have a hard time coming up with a good name if we were to do that.
It seems that it is not a good idea to do such embedding, and if
that is the case, "gitdiff_data" is a fine for the three-field
struct.
Yeah, I think that's the best way forward, thanks.
Thanks.
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help