Jump to conversation
Unresolved conversations (0)
Nice work!

Nice work!

All of your conversations have been resolved.

Resolved conversations (8)
@delvh delvh Sep 27, 2021
Interesting. I expected that you would decode `msg` here to check whether `decodedMsg == msg`. That was to show that these keys indeed work as intended. This implementation surprises me.
modules/activitypub/keypair_test.go
techknowlogick
@delvh delvh Sep 27, 2021
```suggestion assert.NoError(t, err) ```
Outdated
modules/activitypub/keypair_test.go
techknowlogick
@delvh delvh Sep 27, 2021
Can't the if be replaced by its content? I hope `assert.NoError` checks whether an error is present itself, otherwise that function makes only limited sense. Correct me if this function categorically fails the test. ```suggestion assert.NoError(t, err) ```
Outdated
modules/activitypub/keypair_test.go
@delvh delvh Sep 20, 2021
I think an additional statement where you encode and then decode something should be added. This is just to show the integrity of the keys, even though I believe the crypto package already should take care of that.
modules/activitypub/keypair_test.go
techknowlogick
@6543 6543 Sep 19, 2021
a smal simple unit test would be nice :)
modules/activitypub/keypair.go
techknowlogick
@delvh delvh Sep 17, 2021
Is there a reason why this parameter is a generic interface and not a `rsa.PublicKey`? Because while it most likely can accept anything, the only thing I think it should accept is a `rsa.PublicKey`. Everything else is most likely a runtime error. So, the result should most likely be ```suggestion func pemBlockForPub(pub rsa.PublicKey) (string, error) { ``` or ```suggestion func pemBlockForPub(pub *rsa.PublicKey) (string, error) { ```
Outdated
modules/activitypub/keypair.go
@delvh delvh Sep 16, 2021
```suggestion ```
Outdated
modules/activitypub/keypair.go
@delvh delvh Sep 16, 2021
I don't think the `publicKey` function is needed as it is called only here. Additionally, the function declaration seems overly complicated to me. If I understand go syntax correctly, the following should still be valid, right? ```suggestion pubPem, err := pemBlockForPub(&priv.PublicKey) ```
Outdated
modules/activitypub/keypair.go