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

include config dir clearance in clean-app script #3657

Merged
merged 3 commits into from
Dec 10, 2024

Conversation

mallexxx
Copy link
Collaborator

Task/Issue URL: https://app.asana.com/0/1201037661562251/1208943073994046/f

Description:

  • Include ~/Library/Group\ Containers/*.com.duckduckgo.macos.browser.app-configuration.debug/* path to clear by the ./clean-app.sh script

Optional E2E tests:

  • Run PIR E2E tests
    Check this to run the Personal Information Removal end to end tests. If updating CCF, or any PIR related code, tick this.

Steps to test this PR:

  1. Validate downloaded configs directory is cleared by the script for both DMG and AppStore builds

Definition of Done:


Internal references:

Pull Request Review Checklist
Software Engineering Expectations
Technical Design Template
Pull Request Documentation

@mallexxx mallexxx requested a review from ayoy December 10, 2024 12:46
clean-app.sh Outdated
@@ -8,27 +8,32 @@ delete_data() {
if defaults read "${bundle_id}" &>/dev/null; then
defaults delete "${bundle_id}"
fi
rm -rf "${HOME}/Library/Containers/${bundle_id}/Data"
rm -r "${HOME}/Library/Containers/${bundle_id}/Data"
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why are we removing -f? This will break the script when the directory doesn't exist.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

well, it does for me;
but it does report a missing dir status instead if you run it twice

Copy link
Collaborator

Choose a reason for hiding this comment

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

Sorry about that, the script actually goes on (we’re not using set -e here, but displays an error). Would be great to have it handled gracefully if we’re skipping -f. Thanks for updating it.

clean-app.sh Outdated Show resolved Hide resolved
Copy link
Collaborator

@ayoy ayoy left a comment

Choose a reason for hiding this comment

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

Looks and works great, thanks @mallexxx!

@mallexxx mallexxx merged commit 763dbb4 into main Dec 10, 2024
13 of 14 checks passed
@mallexxx mallexxx deleted the alex/clear-config-script branch December 10, 2024 15:04
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