Skip to content
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

Refactor data fetching and filtering logic in get.ts #229

Merged

Conversation

dnafication
Copy link
Contributor

@dnafication dnafication commented Oct 8, 2023

Refactor data fetching and filtering logic in get.ts

This pull request refactors the data/get.ts file to make it more readable and modular. The data fetching logic is now separated from the filtering logic, and reusable utility functions have been extracted into a separate utils.ts file.

The changes include:

  • Renamed data/get.ts to data/index.ts and converted it to use async/await syntax
  • Extracted data fetching logic for github and gitlab into separate functions into their own files
  • Common processing logic lives in the shared.ts file
  • Added a new Data interface to define the shape of the data file to be written
  • Extracting reusable utility functions into a separate utils.ts file
  • Perform write operations for the JSON data file and sitemap data in parallel asynchronously
  • Logs all the tags and languages that are being excluded by the filter
  • Updating function and variable names to be more descriptive
  • Simplifying and optimizing code where possible
  • Added descriptive comments to functions and variables

These changes should make the code easier to read, maintain, and extend in the future.

Please review and let me know if there are any issues or concerns with these changes. Happy to discuss and make any necessary changes.

related to #206 and its discussions

@vercel
Copy link

vercel bot commented Oct 8, 2023

Someone is attempting to deploy a commit to a Personal Account owned by @lucavallin on Vercel.

@lucavallin first needs to authorize it.

types.ts Outdated Show resolved Hide resolved
lucavallin
lucavallin previously approved these changes Oct 9, 2023
Copy link
Owner

@lucavallin lucavallin left a comment

Choose a reason for hiding this comment

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

Great work - thanks for the refactoring!

@dnafication
Copy link
Contributor Author

dnafication commented Oct 9, 2023 via email

@dnafication
Copy link
Contributor Author

@lucavallin Reviews comments have been addressed and the code is ready for merging. Please let me know if there are any additional concerns.

lucavallin
lucavallin previously approved these changes Oct 11, 2023
@vercel
Copy link

vercel bot commented Oct 11, 2023

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
first-issue ✅ Ready (Inspect) Visit Preview 💬 Add feedback Oct 11, 2023 8:01pm

@lucavallin
Copy link
Owner

@dnafication Sorry for the delay! Approved, but there is a conflict.

@dnafication
Copy link
Contributor Author

Resolved conflicts and merged the latest main. Feel free to merge it @lucavallin

Copy link
Owner

@lucavallin lucavallin left a comment

Choose a reason for hiding this comment

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

🥳

Thanks!

@lucavallin lucavallin merged commit 0a4df4e into lucavallin:main Oct 12, 2023
1 check failed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants