-
Notifications
You must be signed in to change notification settings - Fork 5
fix: handle breaking changes to workspace config #349
fix: handle breaking changes to workspace config #349
Conversation
…e-breaking-changes-workspace-config
Pull Request Test Coverage Report for Build 13674149396Details
💛 - Coveralls |
|
||
export function WorkspaceName({ | ||
className, | ||
workspaceName, | ||
workspaceName: oldWorkspaceName, |
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.
why renaming 🤔 ? when we create a new workspace we don't have any oldWorkspaceName
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.
It's just for clarity — here we are in the form to update a workspace name, and whatever we pass will become the new workspace name — in the context of the file I think it makes sense
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.
yep but when you create a new workspace it is a different use case, you don't have any old workspace name, anyways I don't think it is something really important 👍
import { useInvalidateWorkspaceQueries } from './use-invalidate-workspace-queries' | ||
import { useToastMutation } from '@/hooks/use-toast-mutation' | ||
import { useQueryClient } from '@tanstack/react-query' | ||
import { removeQueriesByIds } from '@/lib/react-query-utils' | ||
|
||
export function useMutationCreateWorkspace() { |
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.
it seems not used anymore, after replacing with workspace update, I think we can remove here and later from codegate repo?
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.
We still need the create workspace endpoint — there are 2 basically:
POST /api/v1/workspaces
-> creates a workspace
PUT /api/v1/workspaces/:workspace_name
-> updates a workspace
The crux of the change is that we don't re-use the POST endpoint for renaming.
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.
but i don't see you're using it in these changes 🤔 , am I wrong?
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.
Companion PR for stacklok/codegate#1107