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

Redo of instant messenger section (centralized, federated, peer to peer) #1500

Merged

Conversation

@dngray
Copy link
Member

dngray commented Nov 17, 2019

I needed to add these icons because they did not exist.

The only one I needed to change was the XMPP logo, as it wasn't square, (only by a few pixels).

Matrix

I also edited the description for Matrix. Initially in #1377 I was going to mention about the privacy sprint. I think this might be a bit complex for new users and really unless they understand what those issues were about it serves little purpose in adding it.

Likewise the confusion between RiotX and regular Riot, I decided not to mention this as users should use what is generally available unless they know otherwise.

I also removed the Experimental E2EE badge as I do not believe that the previous description at all reflects the stability of E2EE in Riot currently which is pretty good, just need to remember to enable it.

XMPP

The original description is too complex so I decided to simplify that also. We won't talk about the bad XMPP clients just the ones we've handpicked.


Netlify preview: https://deploy-preview-1500--privacytools-io.netlify.com/software/real-time-communication/

@dngray dngray requested a review from privacytoolsIO/editorial as a code owner Nov 17, 2019
@netlify

This comment has been minimized.

Copy link

netlify bot commented Nov 17, 2019

Deploy preview for privacytools-io ready!

Built with commit b0e178f

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

@dngray dngray changed the title Initial implementation of https://github.com/privacytoolsIO/privacyto… Redo of instant messenger section (centralized, federated, peer to peer) Nov 17, 2019
@dngray

This comment has been minimized.

Copy link
Member Author

dngray commented Nov 17, 2019

https://github.com/dngray/ptio-edited-logos I put up the logos here, they got edited with a border.

Copy link
Member

Mikaela left a comment

I have to pause reviewing to have a dinner, but I will continue from P2P as soon as I can.

_includes/sections/instant-messenger.html Outdated Show resolved Hide resolved
_includes/sections/instant-messenger.html Outdated Show resolved Hide resolved
_includes/sections/instant-messenger.html Outdated Show resolved Hide resolved
_includes/sections/instant-messenger.html Outdated Show resolved Hide resolved
_includes/sections/instant-messenger.html Outdated Show resolved Hide resolved
_includes/sections/instant-messenger.html Outdated Show resolved Hide resolved
_includes/sections/instant-messenger.html Outdated Show resolved Hide resolved
_includes/sections/instant-messenger.html Outdated Show resolved Hide resolved
_includes/sections/instant-messenger.html Outdated Show resolved Hide resolved
_includes/sections/instant-messenger.html Outdated Show resolved Hide resolved
_includes/sections/instant-messenger.html Outdated Show resolved Hide resolved
_includes/sections/instant-messenger.html Outdated Show resolved Hide resolved
_includes/sections/instant-messenger.html Outdated Show resolved Hide resolved
_includes/sections/instant-messenger.html Outdated Show resolved Hide resolved
_includes/sections/instant-messenger.html Outdated Show resolved Hide resolved
Copy link
Contributor

djoate left a comment

Some nitpicks and general comments.

Another nit: There's also lot of text that says what E2EE stands for ("E2EE (end-to-end encryption)"), so it might feel better and less redundant if we just wrote out "end-to-end encryption". The Signal and Wire cards don't use "E2EE", for example

_includes/sections/instant-messenger.html Outdated Show resolved Hide resolved
_includes/sections/instant-messenger.html Outdated Show resolved Hide resolved
_includes/sections/instant-messenger.html Outdated Show resolved Hide resolved
_includes/sections/instant-messenger.html Outdated Show resolved Hide resolved
_includes/sections/instant-messenger.html Outdated Show resolved Hide resolved
_includes/sections/instant-messenger.html Outdated Show resolved Hide resolved
_includes/sections/instant-messenger.html Outdated Show resolved Hide resolved
_includes/sections/instant-messenger.html Outdated Show resolved Hide resolved
_includes/sections/instant-messenger.html Outdated Show resolved Hide resolved
_includes/sections/instant-messenger.html Outdated Show resolved Hide resolved
@dngray dngray self-assigned this Nov 18, 2019
@dngray dngray force-pushed the dngray:pr-instant_messaging_refinement branch from fd7c1a5 to 45d0d70 Nov 18, 2019
@dngray

This comment has been minimized.

Copy link
Member Author

dngray commented Nov 18, 2019

Some nitpicks and general comments.

Another nit: There's also lot of text that says what E2EE stands for ("E2EE (end-to-end encryption)"), so it might feel better and less redundant if we just wrote out "end-to-end encryption". The Signal and Wire cards don't use "E2EE", for example

We explain what E2EE is so I've opted to use that throughout after explaining what the acronym means in the first paragraph 4a368f9 I figured there's no point in having an acronym if we don't use it to make thing shorter.

@dngray dngray force-pushed the dngray:pr-instant_messaging_refinement branch 4 times, most recently from 827ee3f to 514cda7 Nov 19, 2019
@nitrohorse

This comment has been minimized.

Copy link
Member

nitrohorse commented Nov 20, 2019

Also worth noting we'll need to create Discourse forum links for any new suggestions.

Copy link
Member

Mikaela left a comment

Sorry @dngray, more things to address were given to me in private at Matrix

_includes/sections/instant-messenger.html Outdated Show resolved Hide resolved
_includes/sections/instant-messenger.html Outdated Show resolved Hide resolved
@Mikaela Mikaela self-assigned this Nov 25, 2019
@Mikaela

This comment has been minimized.

Copy link
Member

Mikaela commented Nov 25, 2019

Assigning myself, so I maybe will remember to use the fine suggest changes button or may be sending PRs to your branch

@dngray dngray force-pushed the dngray:pr-instant_messaging_refinement branch from 11c34ce to 9b21697 Nov 27, 2019
@Mikaela

This comment has been minimized.

Copy link
Member

Mikaela commented Nov 27, 2019

I added the netlify link to the original comment and I am not sure if the bridge/transport part was removed, but could it be noted that bridges/transports/relays turn federation/P2P into a single point of failure? And is the depedency on your server explained or do we consider that as too obvious?

Copy link
Collaborator

danarel left a comment

Looks great!

@dngray dngray dismissed stale reviews from JonahAragon and danarel via 399565c Nov 28, 2019
@dngray dngray force-pushed the dngray:pr-instant_messaging_refinement branch from b367a25 to 564b2b3 Nov 28, 2019
@dawidpotocki dawidpotocki force-pushed the dngray:pr-instant_messaging_refinement branch from 564b2b3 to 5fc939c Nov 28, 2019
Copy link
Member

Mikaela left a comment

Emergency approval before I leave from home as requested on the team chat, however I disagree with demoting XMPP.

(And my other change requests will probably need a new PR?)

@dngray dngray force-pushed the dngray:pr-instant_messaging_refinement branch from 5fc939c to c93e5c8 Nov 28, 2019
@Mikaela

This comment has been minimized.

Copy link
Member

Mikaela commented Nov 28, 2019

shouldn't Matrix have warning that E2EE is not enabled by default yet and it stores media files forever and the reader needs to ask themselves whether they trust Matrix's encryption to protect them in 5, 10...50 years?

matrix-org/synapse#1263

Copy link
Member

Mikaela left a comment

another emergency approval while I still disagree with the direction this PR took overnight.

@dngray dngray force-pushed the dngray:pr-instant_messaging_refinement branch from e505859 to b0e178f Nov 28, 2019
@JonahAragon

This comment has been minimized.

Copy link
Member

JonahAragon commented Nov 28, 2019

I disagree with demoting XMPP.

Was this even something previously discussed? It was never full-recommended in the first place, this keeps it in the same position.

Copy link
Member

dawidpotocki left a comment

duawdhuawhdawudh

@dngray dngray merged commit 6fac572 into privacytoolsIO:master Nov 28, 2019
6 checks passed
6 checks passed
Header rules No header rules processed
Details
Pages changed All files already uploaded
Details
Redirect rules No redirect rules processed
Details
Mixed content No mixed content detected
Details
Travis CI - Pull Request Build Passed
Details
deploy/netlify Deploy preview ready!
Details
@dngray dngray deleted the dngray:pr-instant_messaging_refinement branch Nov 28, 2019
@Mikaela

This comment has been minimized.

Copy link
Member

Mikaela commented Nov 28, 2019

Clarification on my previous comments:

I woke up at 07.23 which is earlier than usually and left for psykofyysinen kuntoutus at 8.23 (where I arrived late). During this time I did my morning tasks and checked the PrivacyTools team chat where I learned that a blog post has been posted and this needs emergency merging immediately and that we have delisted XMPP.

After I left from home, I was unable to reach other team members than @nitrohorse on XMPP and Wire (and we also established a Signal connection) who apologised for not understanding that I didn't have time for reviewing and clarified to me that we have not delisted XMPP and XMPP is still a worth mentioning app, which I can agree with).

I also learned that as a backup to the teamchat on Matrix, we are having a backup chat on Keybase. However neither works on my main phone (and I was too di/stressed about everything to remember I have a powered off spare phone that Riot works on) as they are too heavy.

I am not sure if RiotX has an issue about not performing well on Nokia 1, but at first it was showing me that everything is an encrypted message, then I maybe got two or three lines sent and read and as it then threw me to ancient history in the scrollback I attempted the clear cache and reload trick, which RiotX never continued further. I am aware that RiotX is an experimental app, but even with its malfunctioning, it works better and seems lighter than Riot.

On Keybase, I have been complaining about it not working on my main phone for almost an year keybase/client#15115 and it's also one of the most heavy apps I run on my desktop (that has 8 GB RAM) alongside Riot, Signal and Wire.

PS. I haven't still reviewed what has happened to this PR since my previous full review yesterday or earlier and I don't see much point doing that seeing that it has been merged already.

PPS. I have no access to either team chat until around 14 UTC as the app is too heavy for my work try-out practice device with 4 GB of RAM that is efficiently eaten by Firefox, RocketChat, Riot and Telegram.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.