Skip to content
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

fixup! fsmonitor: reintroduce core.useBuiltinFSMonitor #3783

Merged
merged 1 commit into from
Apr 11, 2022

Conversation

derrickstolee
Copy link

@derrickstolee derrickstolee commented Apr 11, 2022

This change to warn about core.useBuiltinFSMonitor is a good one, but it
is too aggressive. If a user has set core.useBuiltinFSMonitor=false,
then that warning shows up.

Perhaps there is value in warning users that their explicit disabling of
the feature will stop working. However, VFS for Git does this
assignment, so all users on those enlistments will start getting
warnings after upgrading Git. This situation is probably much more
likely than a typical user disabling the experimental feature
themselves.

Putting in this fix is easier than rereleasing VFS for Git.

This change to warn about core.useBuiltinFSMonitor is a good one, but it
is too aggressive. If a user has set core.useBuiltinFSMonitor=false,
then that warning shows up.

Perhaps there is value in warning users that their explicit disabling of
the feature will stop working. However, VFS for Git does this
assignment, so all users on those enlistments will start getting
warnings after upgrading Git. This situation is probably much more
likely than a typical user disabling the experimental feature
themselves.

Putting in this fix is easier than rereleasing VFS for Git.

Signed-off-by: Derrick Stolee <[email protected]>
@derrickstolee derrickstolee merged commit 00388b6 into git-for-windows:main Apr 11, 2022
@vdye vdye mentioned this pull request Apr 12, 2022
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.

3 participants