Skip to content
This repository has been archived by the owner on Jun 2, 2022. It is now read-only.

chore: Remove hooks folder #377

Merged
merged 4 commits into from
Mar 14, 2022
Merged

chore: Remove hooks folder #377

merged 4 commits into from
Mar 14, 2022

Conversation

tlgimenes
Copy link
Contributor

@tlgimenes tlgimenes commented Mar 8, 2022

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 folder hooks 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

  • CHANGELOG entry added

@netlify
Copy link

netlify bot commented Mar 8, 2022

✔️ 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

@vtex-sites
Copy link

vtex-sites bot commented Mar 8, 2022

Preview is ready

This pull request generated a Preview

👀   Preview: https://preview-377--base.preview.vtex.app
🔬   Go deeper by inspecting the Build Logs
📝   based on commit fb79cd6

@gatsby-cloud
Copy link

gatsby-cloud bot commented Mar 8, 2022

Gatsby Cloud Build Report

basestore

🎉 Your build was successful! See the Deploy preview here.

Build Details

View the build logs here.

🕐 Build time: 9m

Performance

Lighthouse report

Metric Score
Performance 🔶 83
Accessibility 💚 100
Best Practices 💚 100
SEO 💚 93

🔗 View full report

@tlgimenes tlgimenes marked this pull request as ready for review March 8, 2022 18:22
@tlgimenes tlgimenes requested a review from a team March 8, 2022 18:26
Copy link
Contributor

@filipewl filipewl left a 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.

src/components/common/PostalCode/PostalCodeInput.tsx Outdated Show resolved Hide resolved
@filipewl filipewl requested a review from a team March 8, 2022 21:31
Copy link
Member

@eduardoformiga eduardoformiga left a 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 :)

CHANGELOG.md Outdated Show resolved Hide resolved
src/components/common/PostalCode/PostalCodeInput.tsx Outdated Show resolved Hide resolved
src/components/common/PostalCode/PostalCodeInput.tsx Outdated Show resolved Hide resolved
@tlgimenes tlgimenes merged commit fb79cd6 into master Mar 14, 2022
@tlgimenes tlgimenes deleted the chore/homogeneous branch March 14, 2022 18:11
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.

5 participants