Details
I reproduced the issue by following these instructions, but making sure to log in with SIWE. Before this diff the issue reproduces; after the diff, it no longer reproduced
Diff Detail
- Repository
- rCOMM Comm
- Lint
Lint Not Applicable - Unit
Tests Not Applicable
Event Timeline
| native/components/background-identity-login-handler.react.js | ||
|---|---|---|
| 31–37 ↗ | (On Diff #42123) | This sounds confusing: we're checking if we are logged in to identity to know if we are ready to try the identity login. |
| native/components/background-identity-login-handler.react.js | ||
|---|---|---|
| 31–37 ↗ | (On Diff #42123) | Hmm... I see your point. I think it all stems from the naming of isLoggedInToIdentityAndAuthoritativeKeyserver. That name is based on a post-identity world, where currentUserInfo is set by identity (mostly), and the authoritative keyserver cookie is set in keyserverStore by the authoritative keyserver. However, in a pre-identity world, both of these are set by the authoritative keyserver. I'm going to land this for now, but I'm curious if you have a better suggested name for isLoggedInToIdentityAndAuthoritativeKeyserver that could improve the readability here. |
| native/components/background-identity-login-handler.react.js | ||
|---|---|---|
| 31–37 ↗ | (On Diff #42123) | Not sure, we can consider using a different layer of abstraction and say hasCurrentUserInfoAndIsLoggedInAuthoritativeKeyserver - but it is really long. We can also consider mentioning that the user was logged in to identity. But overall, I don't see a name that is significantly better than this. |