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

refactor: remove deprecated "ipfs on PATH" feature #1948

Merged
merged 6 commits into from
Feb 3, 2022
Merged

Conversation

hacdias
Copy link
Member

@hacdias hacdias commented Jan 4, 2022

This PR (almost) completely removes the feature 'IPFS on PATH'. This feature was officially deprecated in #1768 and only stayed active for users that had it enabled.

Even while deprecated, this feature is creating problems (see #1943). This PR completely uninstalls IPFS on PATH for the users that still have it enabled and shows this little message:

image

Fixes #1943
Closes #1969

In a few versions, we should remove the uninstall code. I think it is wise to keep for now so that it reverts the changes that it made before.

@hacdias hacdias requested a review from lidel January 4, 2022 09:24
Copy link
Member

@lidel lidel left a comment

Choose a reason for hiding this comment

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

Thank you for taking time and cleaning this up @hacdias 🙏

I think we should minimize the amount of invasive operations
See inline comments for uninstall on windows and other platforms.

If we adjust, this should be safe to ship.

src/ipfs-on-path/index.js Show resolved Hide resolved
src/ipfs-on-path/index.js Outdated Show resolved Hide resolved
src/ipfs-on-path/scripts/uninstall.js Outdated Show resolved Hide resolved
src/ipfs-on-path/index.js Show resolved Hide resolved
@lidel lidel changed the title refactor: remove ipfs on path refactor: remove deprecated "ipfs on PATH" feature Jan 21, 2022
@hacdias hacdias requested a review from lidel January 21, 2022 13:55
@lidel lidel mentioned this pull request Feb 3, 2022
Copy link
Member

@lidel lidel left a comment

Choose a reason for hiding this comment

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

Thanks!
Merging, as I believe we should ship this.
It does its best to clean up any remaining cruft for legacy users, but does not impact new ones.

(I did not bother to add i18n, assuming CLI users don't mind one-time message in English, since our CLI is only in English anyway)

@lidel lidel merged commit 80ee282 into main Feb 3, 2022
@lidel lidel deleted the remove-ipfs-path branch February 3, 2022 23:28
@hacdias hacdias mentioned this pull request Sep 1, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
No open projects
Archived in project
Development

Successfully merging this pull request may close these issues.

[gui error report] Windows: EPERM %userprofile%\.ipfs-desktop\IPFS_PATH
2 participants