From: Patryk Obara Date: Mon, 16 Mar 2020 12:31:20 GMT Subject: Re: [PATCH 2/2] dir: improve naming of oid_stat fields in two structs Message-ID: <938146aa-e095-66fb-da0b-10e8796eaa12@gmail.com> In-Reply-To: On 16/03/2020 07:17, Junio C Hamano wrote: > Matheus Tavares writes: > >> Note: I choosed to use "st_*", as naming, for simplicity, and to keep >> the code lines small. But should we use a more verbose "oidst_*" format, >> instead, to avoid confusions with "struct stat"? >> ... >> @@ -334,8 +334,8 @@ struct dir_struct { >> >> /* Enable untracked file cache if set */ >> struct untracked_cache *untracked; >> - struct oid_stat ss_info_exclude; >> - struct oid_stat ss_excludes_file; >> + struct oid_stat st_info_exclude; >> + struct oid_stat st_excludes_file; >> unsigned unmanaged_exclude_files; >> }; > > I tend to agree with you that using prefix "st_" for anything other > than "struct stat" proper would be confusing. If "ss" used to stand > for "sha1 stat", I would expect "oid stat" to abbreviate to "os", at > least. I think I stopped myself from more renames in that patch series, because the number of touched lines across various functions started to grow too fast and I was already stepping on brian's toes when touching oid conversions. > I also am wondering if we can do without any prefix, i.e. just call > them "info_exclude" and "excludes_file", because the field names are > private to each struct and there is no strong reason to have a > common prefix among fields in a single struct. Rather, it is more > important for the fields of the same type in a single struct to have > distinct names so that the reader can easily tell them apart and the > reason for their use is straight-forward to understand, and the two > names without any prefix convey their distinction pretty well, I > would think. The evolution of this name seems to be an artefact of refactorization process, and not really a design decision: info_exclude_sha1 (unsigned char[20]) changed to: ss_info_exclude (struct sha1_stat) changed to: ss_info_exclude (struct oid_stat) There are some comments in dir.h still referring to info_exclude_sha1. Therefore removing the prefix altogether (and fixing outdated comment!) would be very welcome. On the other hand, naming them sss_info_exclude or sos_info_exclude would be quite funny, although I don't find hilarity to be a desirable quality for a naming convention ;) -- | ← Ceci n'est pas une pipe Patryk Obara