signalapp / libsignal

Home to the Signal Protocol as well as other cryptographic primitives which make Signal possible.
GNU Affero General Public License v3.0
3.62k stars 420 forks source link

add docs to identity_key.rs #470

Closed cosmicexplorer closed 2 years ago

cosmicexplorer commented 2 years ago

Broken out of #287.

jrose-signal commented 2 years ago

Since the last round of PRs we've changed libsignal to match the other Signal repositories in developing mostly in private and doing public releases, with external contributions being cherry-picked into the upcoming release's private branch. That's not so great for an ongoing patch series like this one, though. Perhaps we can make a side branch your PRs are targeted to, docs-effort or something, so that you can build on your own work between releases? While meanwhile they are also cherry-picked to our private main.

clauz9 commented 2 years ago

Since the last round of PRs we've changed libsignal to match the other Signal repositories in developing mostly in private and doing public releases,

I was just admiring the fact that the development for libsignal is a lot more transparent, and publicly accessible, compared to the clients 🤔 I love the fact that I am able to see developers making PR's for features and reviewing each other's code, it's incredibly educational.

so, can I ask, why the change ?

jrose-signal commented 2 years ago

There wasn't one big reason, but a bunch of small things adding up:

I'm a little sad too, but ultimately while it was nice to have a "fully open" development, that's not really the main goal of the project. The thing we optimize for is "supporting the Signal client apps" rather than "making a model open-source library", and I do think that's the right choice.

[This is not something that can be argued with, but if you want to keep discussing it please move that discussion to https://community.signalusers.org, so we can go back to talking about the PR content and how to integrate it!]

clauz9 commented 2 years ago

Signal employees have to remember to be more circumspect on libsignal reviews (i.e. not mentioning possible future features or security concerns that could get taken out of context, which has historically been quite frustrating)

yes, this makes perfect sense..

This is not something that can be argued with, but if you want to keep discussing it please move that discussion to https://community.signalusers.org, 

No need, your answer was quite detailed, thanks for taking the time to write it!

cosmicexplorer commented 2 years ago

I have updated the description in the tracking issue #467 to note that I am maintaining a branch named docs-effort at https://github.com/cosmicexplorer/libsignal-client/tree/docs-effort with all these changes together! @jrose-signal I appreciate your careful description of the new review process and appreciate all the effort Signal puts into ensuring the app remains secure!

jrose-signal commented 2 years ago

Cherry-picked as 9ad236264d1f0261e0efd714f3c4fa565d4af072, will be in the next release!