-
Notifications
You must be signed in to change notification settings - Fork 896
feat: add extra workspace actions in the workspaces table #17775
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
Changes from 1 commit
496473f
22759e3
2f3cce0
f7e9ba9
a3c4908
d5ae7e1
0c65d4f
3b5f20b
207094a
c79952b
be967dd
139548c
84b65a8
96a6ffd
62413e0
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
- Loading branch information
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -221,8 +221,9 @@ const DownloadingItem: FC<DownloadingItemProps> = ({ file, giveUpTimeMs }) => { | |
function humanBlobSize(size: number) { | ||
const BLOB_SIZE_UNITS = ["B", "KB", "MB", "GB", "TB"] as const; | ||
let i = 0; | ||
while (size > 1024 && i < BLOB_SIZE_UNITS.length) { | ||
size /= 1024; | ||
let sizeIterator = size; | ||
while (sizeIterator > 1024 && i < BLOB_SIZE_UNITS.length) { | ||
sizeIterator /= 1024; | ||
Comment on lines
+224
to
+226
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There's nothing wrong with the changes you made here, but if we're dealing with numbers that are big enough that we're expecting to need to divide it by 1024 multiple times, do we want to update anything to involve There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It is only to humanize the number, so it is a fixed value of 1024 |
||
i++; | ||
} | ||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -50,14 +50,13 @@ const WorkspaceParametersPage: FC = () => { | |
const template = templateQuery.data; | ||
|
||
// Permissions | ||
const checks = | ||
workspace && template ? workspaceChecks(workspace, template) : {}; | ||
const checks = workspace && template ? workspaceChecks(workspace) : {}; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If we're only using the workspace to define the checks, do we still need There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Not at all! There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. One less query 👏 |
||
const permissionsQuery = useQuery({ | ||
...checkAuthorization({ checks }), | ||
enabled: workspace !== undefined && template !== undefined, | ||
}); | ||
const permissions = permissionsQuery.data as WorkspacePermissions | undefined; | ||
const canChangeVersions = Boolean(permissions?.updateTemplate); | ||
const canChangeVersions = Boolean(permissions?.updateWorkspaceVersion); | ||
|
||
return ( | ||
<> | ||
|
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.
Nice catch
Uh oh!
There was an error while loading. Please reload this page.
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'm not the biggest fan of changing parameters either, but in this case, this would never be a bug, since numbers are primitive values
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 have to say it was a Biome catch haha.