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
No Lint Coverage - Unit
No Test Coverage
Event Timeline
| native/components/background-identity-login-handler.react.js | ||
|---|---|---|
| 31–37 | 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 | 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 | 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. | |