Skip to content

Conversation

@EliahKagan
Copy link
Member

This renames the crate-internal static item with the cached path of the highest scope nonempty config, treated as associated with the git installation, from GIT_HIGHEST_PRIORITY_CONFIG_PATH to GIT_HIGHEST_SCOPE_CONFIG_PATH.

This reflects the propensity of "priority" to have the opposite of the intended meaning here, since configuration in this file stands to be superseded by configuration in any lower (i.e. any other) scope. See #1568 for context.

I think this change from "PRIORITY" to "SCOPE" in the name preserves the benefits of the change from EXE_INFO to GIT_HIGHEST_PRIORITY_CONFIG_PATH (dd2d666), while making clearer what is going on and avoiding misinterpretations.

This renames the crate-internal static item with the cached path
of the highest scope nonempty config, treated as associated with
the `git` installation, from `GIT_HIGHEST_PRIORITY_CONFIG_PATH` to
`GIT_HIGHEST_SCOPE_CONFIG_PATH`.

This reflects the propensity of "priority" to have the opposite of
the intended meaning here, since configuration in this file stands
to be superseded by configuration in any lower (i.e. any other)
scope. See GitoxideLabs#1568 for context.

I think this change from "PRIORITY" to "SCOPE" in the name
preserves the benefits of the change from `EXE_INFO` to
`GIT_HIGHEST_PRIORITY_CONFIG_PATH` (dd2d666), while making clearer
what is going on and avoiding misinterpretations.
Copy link
Member

@Byron Byron left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks a lot - naming is hard 🤦‍♂️

@Byron Byron merged commit 3cf9694 into GitoxideLabs:main Aug 31, 2024
@EliahKagan
Copy link
Member Author

Thanks a lot - naming is hard 🤦‍♂️

Yes, and I'm not sure this one is the best possible name either. If I think of something better, I'll let you know!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants