-
Notifications
You must be signed in to change notification settings - Fork 5
Conversation
✔️ Deploy Preview for basestore ready! 🔨 Explore the source changes: 1f9fd71 🔍 Inspect the deploy log: https://app.netlify.com/sites/basestore/deploys/622f8271d20da50008766913 😎 Browse the preview: https://deploy-preview-377--basestore.netlify.app |
Preview is readyThis pull request generated a Preview👀 Preview: https://preview-377--base.preview.vtex.app |
Gatsby Cloud Build Reportbasestore 🎉 Your build was successful! See the Deploy preview here. Build Details🕐 Build time: 9m PerformanceLighthouse report
|
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.
LGTM! I remember a previous comment where you mentioned this need to move everything from /hook
to /sdk
.
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.
LGTM! Moving the useSearchParams.ts
inline looks harder for readability. But since we just use it on one file and it's good to improve the codebase size, I agree with this change :)
c157eab
to
32bbc42
Compare
32bbc42
to
2fe8173
Compare
7967dd8
to
1f9fd71
Compare
What's the purpose of this pull request?
This PR removes the hooks folder.
How does it work?
The main idea of the
sdk
folder is to contain hooks with business logic that are used across the app. Turns out a new folderhooks
was created, which lead to duplication of purposes on the filesystem.This PR unifies both hooks and sdk folders. Initially, I thought about moving the files "as is" from hooks to sdk, however, a closer inspection revealed that hooks on the
hooks
folder were only being used in a single file. I then, decided to inline these functions with the intent of having a smaller code base.How to test it?
Everything should work as before
Checklist