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.