Skip to content

Conversation

rkm
Copy link
Contributor

@rkm rkm commented Nov 2, 2020

From testing, I found that the bin and obj directories may be elsewhere depending on the particular hook project layout. I'd also like to cleanup the build directory properly, since we only need the dotnetenv-default dir after we install the tool.

This patch should solve both cases.

@rkm rkm force-pushed the dotnet-fix-cleanup branch 3 times, most recently from 8e9ae98 to 8bfa650 Compare November 2, 2020 17:35
if os.path.isfile(path):
os.remove(path)
elif f not in to_keep:
rmtree(path)
Copy link
Member

Choose a reason for hiding this comment

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

the problem with this is it will break when introducing language_version for dotnet as that allows multiple builds in a single clone

is there a way we can leverage git clean to do this for us?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah, good point -- I had hoped we could blitz the entire git directory after the tool install.

I'll look for a git cleaner solution

Copy link
Member

Choose a reason for hiding this comment

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

there's also the potential that a script hook is defined in the same repo as well

@rkm rkm force-pushed the dotnet-fix-cleanup branch from 8bfa650 to 76c5950 Compare November 2, 2020 18:06
@rkm rkm requested a review from asottile November 2, 2020 18:07
prefix,
(
'git', 'clean', '-fxd',
'-e', helpers.environment_dir(ENVIRONMENT_DIR, version),
Copy link
Member

Choose a reason for hiding this comment

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

this should probably be f'{ENVIRONMENT_DIR}-*'

helpers.run_setup_cmd(
prefix,
(
'git', 'clean', '-fxd',
Copy link
Member

Choose a reason for hiding this comment

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

I've actually got a video coming up soon that indicates this should be -ffxd :)

@rkm rkm force-pushed the dotnet-fix-cleanup branch from 76c5950 to 2b12203 Compare November 2, 2020 18:36
@rkm rkm requested a review from asottile November 3, 2020 09:08
Copy link
Member

@asottile asottile left a comment

Choose a reason for hiding this comment

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

@asottile asottile merged commit 392a1fe into pre-commit:master Nov 3, 2020
@rkm rkm deleted the dotnet-fix-cleanup branch December 6, 2022 12:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants