-
Notifications
You must be signed in to change notification settings - Fork 1.4k
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 our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat: rename access tokens to api keys #6687
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.
👍 Looks good to me! Reviewed everything up to 594c2f1 in 32 seconds
More details
- Looked at
298
lines of code in9
files - Skipped
0
files when reviewing. - Skipped posting
2
drafted comments based on config settings.
1. frontend/src/container/APIKeys/APIKeys.tsx:574
- Draft comment:
The title of the delete modal should be 'Delete Key' instead of 'Delete Token' to maintain consistency with the rest of the application. - Reason this comment was not posted:
Comment was not on a valid diff hunk.
2. frontend/src/container/APIKeys/APIKeys.tsx:525
- Draft comment:
Use design tokens or predefined color constants instead of hardcoding color values. This issue is also present on lines 574, 612, 683, and 704. - Reason this comment was not posted:
Comment was not on a valid diff hunk.
Workflow ID: wflow_FQR7UclpETLmpegT
You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet
mode, and more.
420c9cb
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.
👍 Looks good to me! Incremental review on 420c9cb in 1 minute and 11 seconds
More details
- Looked at
27
lines of code in2
files - Skipped
0
files when reviewing. - Skipped posting
2
drafted comments based on config settings.
1. frontend/src/mocks-server/__mockdata__/apiKeys.ts:85
- Draft comment:
Consider renaming 'No Expiry Key' to 'No Expiry API Key' for consistency with the new terminology. This applies to other instances like 'Editor Token for user 1', 'Editor Token for user 2', etc. - Reason this comment was not posted:
Decided after close inspection that this draft comment was likely wrong and/or not actionable:
The comment points out a real inconsistency in naming - some entries use "Token" while others use "Key". However, this seems more like a suggestion than a critical issue. The comment also goes beyond the scope of the current change by suggesting changes to other entries that weren't modified. The rules say to ignore cross-file issues and only focus on the changed code.
The inconsistency in naming could cause confusion for users and developers. Having a consistent terminology throughout the UI is important for user experience.
While naming consistency is good, this comment goes beyond the scope of the current change and feels more like a nice-to-have suggestion rather than a critical issue that needs to be fixed now.
Delete this comment. While it raises a valid point about consistency, it goes beyond the scope of the current change and isn't critical enough to block this PR.
2. frontend/src/container/APIKeys/APIKeys.tsx:547
- Draft comment:
Use design tokens or predefined color constants instead of hardcoding color values for consistency. This applies to other instances in this file as well. - Reason this comment was not posted:
Comment was not on a valid diff hunk.
Workflow ID: wflow_NlycBLJRRXLnz6CD
You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet
mode, and more.
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.
👍 Looks good to me! Incremental review on 71fbaa8 in 31 seconds
More details
- Looked at
13
lines of code in1
files - Skipped
0
files when reviewing. - Skipped posting
2
drafted comments based on config settings.
1. frontend/src/container/APIKeys/APIKeys.tsx:552
- Draft comment:
Consider renamingisExpiredToken
toisExpiredAPIKey
for consistency with the terminology change. This applies to other instances in the file as well. - Reason this comment was not posted:
Comment was not on a valid diff hunk.
2. frontend/src/container/APIKeys/APIKeys.tsx:555
- Draft comment:
Avoid using inline styles in React components. Use external stylesheets, CSS classes, or styled components instead. This is applicable in multiple places in this file. - Reason this comment was not posted:
Comment was on unchanged code.
Workflow ID: wflow_zcvGskW0B8ThhVUV
You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet
mode, and more.
71fbaa8
to
a9a35f6
Compare
Revert : #4597
Screen.Recording.2024-12-20.at.17.15.24.mov
Important
Renames 'Access Tokens' to 'API Keys' across text, routes, and components for consistency.
routes.json
,titles.json
, andapiKeys.json
.APIKeys.tsx
andAPIKeys.test.tsx
to reflect the new terminology.routes.ts
andAppRoutes/routes.ts
from/settings/access-tokens
to/settings/api-keys
.APIKeys
component inAPIKeys.tsx
to use 'API Keys' in UI elements and logic.APIKeys.test.tsx
to align with the new terminology.This description was created by for 71fbaa8. It will automatically update as commits are pushed.