-
Notifications
You must be signed in to change notification settings - Fork 5
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
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