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

gitindex: include environ for git tests #760

Merged
merged 2 commits into from
Apr 21, 2024
Merged

gitindex: include environ for git tests #760

merged 2 commits into from
Apr 21, 2024

Conversation

keegancsmith
Copy link
Member

A recent change introduced GIT_CONFIG_GLOBAL and GIT_CONFIG_SYSTEM, but ran into the footgun of unsetting the rest of the environ when cmd.Env is non-empty. This for example broke tests for me since we didn't have my custom $PATH set.

Instead we revert the change and make a much smaller change to just always set the relevant environment variables.

Test Plan: go test

A recent change introduced GIT_CONFIG_GLOBAL and GIT_CONFIG_SYSTEM, but
ran into the footgun of unsetting the rest of the environ when cmd.Env
is non-empty. This for example broke tests for me since we didn't have
my custom $PATH set.

Instead we revert the change and make a much smaller change to just
always set the relevant environment variables.

Test Plan: go test
@keegancsmith keegancsmith requested review from camdencheek and a team April 19, 2024 12:30
@cla-bot cla-bot bot added the cla-signed label Apr 19, 2024
Copy link
Member

@camdencheek camdencheek left a comment

Choose a reason for hiding this comment

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

LGTM!

@keegancsmith keegancsmith merged commit 8619902 into main Apr 21, 2024
9 checks passed
@keegancsmith keegancsmith deleted the k/gitindex branch April 21, 2024 08:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants