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

fix: Fix to delete ~/.finch when uninstalling finch #703

Conversation

haytok
Copy link
Member

@haytok haytok commented Nov 26, 2023

In the current implementation, uninstalling finch does not delete the ~/.finch directory.

This directory may hold several GB of container data disks, which can consume a lot of space and be left unremoved.

This issue has also been reported in issue/#457 and needs to be fixed.

Therefore, this fix will ensure that when finch is uninstalled, ~/.finch is also deleted to free up disk space for finch.

Issue #, if available: issue/#457

Description of changes: Details are described in commit messages.

Testing done: Yes

  • I've reviewed the guidance in CONTRIBUTING.md

License Acceptance

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

In the current implementation, uninstalling finch does not delete the
~/.finch directory.

This directory may hold several GB of container data disks, which can
consume a lot of space and be left unremoved.

This issue has also been reported in issue/#457 and needs to be fixed.

Therefore, this fix will ensure that when finch is uninstalled, ~/.finch
is also deleted to free up disk space for finch.

Signed-off-by: Hayato Kiwata <haytok@amazon.co.jp>
@weikequ
Copy link
Contributor

weikequ commented Nov 30, 2023

Thanks for the contribution! I think since this deals with users' persistent data, it would be better to have it opt in instead when uninstalling, or at least give a prompt for if the user would like to keep their data.

Signed-off-by: Hayato Kiwata <haytok@amazon.co.jp>
Copy link
Member Author

@haytok haytok left a comment

Choose a reason for hiding this comment

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

Hi, @weikequ

Thanks for the review !!!
I have changed the code in response to your review, so could you please check back when you have time?

@KevinLiAWS KevinLiAWS merged commit 8d7389f into runfinch:main Dec 5, 2023
13 checks passed
@haytok haytok deleted the clean-up-unused-directory-when-uninstalling-finch branch December 6, 2023 05:41
KevinLiAWS pushed a commit that referenced this pull request Dec 7, 2023
🤖 I have created a release *beep* *boop*
---


## [1.0.1](v1.0.0...v1.0.1)
(2023-12-07)


### Bug Fixes

* Clean up all previous finch version installation registries in
postinstall and uninstall
([#688](#688))
([9afc0b9](9afc0b9))
* Fix to be able to run finch build with --ssh option
([#696](#696))
([4d1e6cf](4d1e6cf))
* Fix to delete ~/.finch when uninstalling finch
([#703](#703))
([8d7389f](8d7389f))
* remove virtual machine image when running make clean
([98c8ee4](98c8ee4))
* resolve shellcheck warnings
([#684](#684))
([d9f695a](d9f695a))
* Use LimaUser method instead of host user name
([#712](#712))
([7c02e08](7c02e08))

---
This PR was generated with [Release
Please](https://github.com/googleapis/release-please). See
[documentation](https://github.com/googleapis/release-please#release-please).

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
KevinLiAWS added a commit that referenced this pull request Dec 11, 2023
🤖 I have created a release *beep* *boop*
---


## [1.0.1](v1.0.0...v1.0.1)
(2023-12-11)


### Bug Fixes

* Change the default behavoir for deleting .finch folder to false when
uninstall ([#732](#732))
([e818743](e818743))
* Clean up all previous finch version installation registries in
postinstall and uninstall
([#688](#688))
([9afc0b9](9afc0b9))
* Fix to be able to run finch build with --ssh option
([#696](#696))
([4d1e6cf](4d1e6cf))
* Fix to delete ~/.finch when uninstalling finch
([#703](#703))
([8d7389f](8d7389f))
* remove virtual machine image when running make clean
([98c8ee4](98c8ee4))
* resolve shellcheck warnings
([#684](#684))
([d9f695a](d9f695a))
* Use LimaUser method instead of host user name
([#712](#712))
([7c02e08](7c02e08))

### Reverts

* "fix: resolve shellcheck warnings"
([#725](#725))
([8ea255a](8ea255a))

---
This PR was generated with [Release
Please](https://github.com/googleapis/release-please). See
[documentation](https://github.com/googleapis/release-please#release-please).

Signed-off-by: Kevin Li <cnkevin@amazon.com>
Co-authored-by: Kevin Li <cnkevin@amazon.com>
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.

None yet

3 participants