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

feat: remove ipfs-desktop settings #1445

Merged
merged 2 commits into from
Apr 21, 2020
Merged

Conversation

hacdias
Copy link
Member

@hacdias hacdias commented Apr 18, 2020

This is ipfs-webui part of ipfs/ipfs-desktop#1425

License: MIT
Signed-off-by: Henrique Dias <hacdias@gmail.com>
@hacdias hacdias marked this pull request as ready for review April 18, 2020 17:39
@hacdias hacdias requested a review from lidel April 18, 2020 17:39
License: MIT
Signed-off-by: Henrique Dias <hacdias@gmail.com>
@lidel
Copy link
Member

lidel commented Apr 21, 2020

@hacdias I see we keep the code related to Countly:
do you remember how that works, or can link issues where it was discussed?

Would be useful for @rafaelramalho19 to be aware how gathering opt-in metrics works.

@hacdias
Copy link
Member Author

hacdias commented Apr 21, 2020

@lidel @rafaelramalho19 yes, we are keeping the Countly code so we can use the same ID for the Web UI when it is used via IPFS Desktop. Issues and PRs:

The latter introduced the sharing between IPFS Desktop and IPFS Web UI where we can use Web UI to enable/disable the analytics.

However, I don't mind removing the Countly-related parts. As far as I remember, we set up this sharing to be able to have better understanding of how IPFS Desktop users were using Web UI, even though we never did that analysis.

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! Let's leave Countly as-is for now, the key here is to remove Desktop settings from Webui, so user always gets the same thing, no matter from where webui was opened from.

@hacdias
Copy link
Member Author

hacdias commented Apr 21, 2020

Thanks for the approval. I'm merging this now then. @rafaelramalho19 if you need something or some explanation about something I did, just ping me.

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