Skip to content
This repository has been archived by the owner on Sep 6, 2024. It is now read-only.

Feat/delete schema #61

Merged
merged 2 commits into from
Aug 8, 2024
Merged

Feat/delete schema #61

merged 2 commits into from
Aug 8, 2024

Conversation

jamieknight-db
Copy link
Collaborator

@jamieknight-db jamieknight-db commented Aug 7, 2024

Screen.Recording.2024-08-07.at.2.59.45.PM.mov

Closes #36

@jamieknight-db
Copy link
Collaborator Author

@xx-db I'm tagging you in a comment in hopes that it lets me add you as a reviewer 🙏

}

export function useDeleteSchema({
onSuccessCallback,
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry I just captured this.
You don't need to do create a new prop here to handle on success.

You can just simply do onSuccess in the caller

mutation.mutate(values, {
  onSuccess: () => {}
});

Can we have follow-up PRs to remove this pattern on other deletion hooks?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you mean to move the invalidateQueries to the caller as well?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nope, keep invalidateQueries in the mutation hook.
It would works for both cases

Copy link
Collaborator

@yc-shawn yc-shawn Aug 7, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Similar example at L39 and L83 from official github

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok got it, but we don't need to pass in onSuccessCallback. I will do a follow-on PR to remove all those. Thanks @yc-shawn 👍

Copy link
Contributor

@xx-db xx-db left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm 💪

@jamieknight-db jamieknight-db merged commit c8d6ec6 into main Aug 8, 2024
1 check passed
@jamieknight-db jamieknight-db deleted the feat/delete-schema branch August 8, 2024 19:45
rtyler pushed a commit to rtyler/unitycatalog that referenced this pull request Sep 6, 2024
* delete schema functionality

* prettier
rtyler pushed a commit to rtyler/unitycatalog that referenced this pull request Sep 6, 2024
* delete schema functionality

* prettier
dennyglee pushed a commit to unitycatalog/unitycatalog that referenced this pull request Sep 6, 2024
* delete schema functionality

* prettier
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Delete Schema
4 participants