- Notifications
You must be signed in to change notification settings - Fork928
feat: Add SSH Keys page on /preferences/ssh-keys#1478
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
I think renamingpreferences tosettings would be helpful. That's the name we'll use in workspaces, so it's better to stay consistent!
| | { type: "GET_SSH_KEY" } | ||
| | { type: "REGENERATE_SSH_KEY" } | ||
| | { type: "CONFIRM_REGENERATE_SSH_KEY" } | ||
| | { type: "CANCEL_REGENERATE_SSH_KEY" } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Should we rename thisUserEvent instead? Seems like it's more accurate.
@kylecarbs Right now the page is called "Preferences" so I think makes more sense to keep using "preferences" than "settings". Settings is something more related to an "account" than a "user" thing, makes sense? And about the event type being UserEvent makes sense but I would like to keep using the implicit convention we are using to name the event as {ServiceName}Event. I have no strong opinion tho. Please let me know what you think so I can make or not the changes. |
GitHub and GitLab both use the term settings, which is my primary reason for preferring that. Just because it's already called preferences doesn't necessarily mean it's right! It's an easy enough change, so if we feel it's clearer to the user, we should make it. |
Hm.. interesting. Working on it! |
Closes#752 and#749