Skip to content
This repository has been archived by the owner on Jun 24, 2022. It is now read-only.

instant-messenger: change Riot privacy conern link to #1049 #1050

Closed
wants to merge 3 commits into from
Closed

instant-messenger: change Riot privacy conern link to #1049 #1050

wants to merge 3 commits into from

Conversation

Mikaela
Copy link
Contributor

@Mikaela Mikaela commented Jul 24, 2019

No description provided.

@Mikaela Mikaela requested a review from a team July 24, 2019 22:48
@netlify
Copy link

netlify bot commented Jul 24, 2019

Deploy preview for privacytools-io ready!

Built with commit eb0221b

https://deploy-preview-1050--privacytools-io.netlify.com

@erciccione
Copy link

I tend to be in favour of this PR and against #1047. But i have to catch up on the discussions before giving a fully informed opinion.

@Mikaela
Copy link
Contributor Author

Mikaela commented Jul 30, 2019

I have removed the description from this PR and the first commit as this appears to be more popular PR and in another commit I fixed the tooltip text that was leftover from the old link.

My personal preference would still be delisting Riot until they have gotten their privacy issues resolved and then listing them again.

Copy link
Contributor

@nitrohorse nitrohorse left a comment

Choose a reason for hiding this comment

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

My personal preference would still be delisting Riot until they have gotten their privacy issues resolved and then listing them again.

I agree. Based on the issues listed in https://github.com/privacytoolsIO/privacytools.io/issues/1049 I think we should remove Riot for now. We could also bump it to the "Worth Mentioning" section with the "privacy conerns" tooltip warning users but that wouldn’t be my first choice.

@Perelandra0x309
Copy link
Contributor

Perelandra0x309 commented Jul 31, 2019

You are missing a closing element on your second anchor link:
<a href="https://github.com/privacytoolsIO/privacytools.io/issues/1049\"<span

@Mikaela
Copy link
Contributor Author

Mikaela commented Jul 31, 2019

I agree. Based on the issues listed in #1049 I think we should remove Riot for now.

Do I understand correctly that you are requesting changes to block merging of this and will be giving an approving review to https://github.com/privacytoolsIO/privacytools.io/pull/1047 ?

We could also bump it to the "Worth Mentioning" section with the "privacy conerns" tooltip warning users but that wouldn’t be my first choice.

I disagree as Riot is not an instant messenger for people, but teams, and would be blocked by https://github.com/privacytoolsIO/privacytools.io/issues/1065.

You are missing a closing element on your second anchor link:
<a href="https://github.com/privacytoolsIO/privacytools.io/issues/1049\"<span

Could you clarify what is wrong in the file and what should it be instead? I looked at it a few times, but I don't spot the issue. I find the code too confusing :(

@nitrohorse
Copy link
Contributor

nitrohorse commented Jul 31, 2019

Do I understand correctly that you are requesting changes to block merging of this and will be giving an approving review to #1047 ?

Yeah 👍🏼 I’ll make an approving review of 1047.

I disagree as Riot is not an instant messenger for people, but teams, and would be blocked by #1065.

Ah I see, that makes sense.

@Perelandra0x309
Copy link
Contributor

Perelandra0x309 commented Jul 31, 2019

Could you clarify what is wrong in the file and what should it be instead? I looked at it a few times, but I don't spot the issue. I find the code too confusing :(

I think it's actually an inherited error from the existing version, but the html should be this:

<a href="https://github.com/privacytoolsIO/privacytools.io/issues/1049\"><span

Add a ">" before the "<span" so it is valid HTML.

Copy link
Contributor

@jonaharagon jonaharagon left a comment

Choose a reason for hiding this comment

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

I would be fine merging this immediately if the syntax issues mentioned here were fixed, although I'm equally fine with not fixing this if we were to merge #1047 instead.

_includes/sections/instant-messenger.html Outdated Show resolved Hide resolved
@Mikaela Mikaela deleted the riot-tracking branch August 7, 2019 15:33
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants