-
Notifications
You must be signed in to change notification settings - Fork 2.4k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Rename options initialization functions #5101
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just a nitpick. I'm 👍 on the normalization.
tests/diff/blob.c
Outdated
@@ -520,19 +520,19 @@ void test_diff_blob__can_compare_a_binary_blob_and_a_text_blob(void) | |||
* +++ b/a0f7217 | |||
* @@ -1,6 +1,6 @@ | |||
* Here is some stuff at the start | |||
* | |||
* |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Spurious whitespace change ? The build was red, but I haven't yet looked at the build log.
069db35
to
dbf9951
Compare
Thanks, I forgot to build locally with hard deprecation enabled. |
dbf9951
to
56c6c50
Compare
Hey @pks-t, are you also 👍 on renaming these? I think that they're more "correct" this way, but naming is pretty subjective. |
I'm 👍 Those names have bothered me several times, so the change is very welcome. You forgot to convert one instance in "src/proxy.c", otherwise this looks good to me! |
I'm not seeing it, I only see |
|
In libgit2 nomenclature, when we need to verb a direct object, we name a function `git_directobject_verb`. Thus, if we need to init an options structure named `git_foo_options`, then the name of the function that does that should be `git_foo_options_init`. The previous names of `git_foo_init_options` is close - it _sounds_ as if it's initializing the options of a `foo`, but in fact `git_foo_options` is its own noun that should be respected. Deprecate the old names; they'll now call directly to the new ones.
56c6c50
to
c0dd712
Compare
Aha - yes! I missed that twice. Thanks, and fixed. |
Rename opt init functions to
options_init
In libgit2 nomenclature, when we need to verb a direct object, we name a function
git_directobject_verb
. Thus, if we need to init an options structure namedgit_foo_options
, then the name of the function that does that should begit_foo_options_init
.The previous names of
git_foo_init_options
is close - it sounds as if it's initializing the options of afoo
, but in factgit_foo_options
is its own noun that should be respected.Deprecate the old names; they'll now call directly to the new ones.
Additionally, add an initializer function for
git_apply_options
.