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

[SPIKE] Refactoring #18

Closed
wants to merge 20 commits into from
Closed

[SPIKE] Refactoring #18

wants to merge 20 commits into from

Conversation

domoscargin
Copy link
Contributor

@domoscargin domoscargin commented Dec 19, 2024

Some hasty, untested refactoring:

  • Moved error handling into its own helper file. The cut is not clean - there's some bits of error handling in the script itself which I didn't get round to factoring out
  • Moved octokit into its own helper and created some functions for the actions we want to do. Again, it's not the cleanest cut
  • Batch write data to disk to avoid lots of I/O operations
  • Create a function for analysing a single repo, then call that function in the loop

I'm pushing and testing this now and I'm pretty sure it'll fail because there's quite a lot of changes. Let's see how it goes!

fixes #12

@domoscargin domoscargin force-pushed the bk-refactoring branch 3 times, most recently from 2400a54 to 236121d Compare December 19, 2024 21:29
@domoscargin domoscargin force-pushed the bk-refactoring branch 9 times, most recently from 0486854 to 7c0a6bb Compare December 20, 2024 00:11
@domoscargin
Copy link
Contributor Author

domoscargin commented Dec 20, 2024

Things I want to do:

  • Investigate using a GraphQL query to grab the repo metadata and relevant package.json/yarn/lockfiles in one fell swoop.
  • Better separate Octokit from the main script
  • Handle errors better
  • Figure out how to handle multiple package.json files (in a monorepo, for instance)
  • Streamline lockfile reading (we only read it when we know there's going to be a packages entry)
  • Take the logic I have for getting parent dependents and add 'em to the output data
  • Ditch streaming

@domoscargin domoscargin force-pushed the bk-refactoring branch 9 times, most recently from b2814b5 to 5a77de6 Compare December 27, 2024 20:30
@domoscargin domoscargin force-pushed the bk-refactoring branch 3 times, most recently from 1712583 to bc8c164 Compare December 27, 2024 20:59
@domoscargin domoscargin force-pushed the bk-refactoring branch 6 times, most recently from f9b7a22 to d23a809 Compare December 31, 2024 22:27
The idea is that if we find one of these packages as a direct dependency, we can infer govuk-frontend's version.

This would rely on a function which checks which package manager we're dealing with in the first instance, and therefore which files to check for dependencies.

We could include manual ports here, and potentially run separate dependents data for them to get an idea of who's using the ports that don't use our package underlying and how much of a market there is.
This has a separate rate limit, and getting this information basically doesn't impact it at all. Should save a few thousand REST API calls.
@domoscargin domoscargin changed the title Refactoring [SPIKE] Refactoring Jan 3, 2025
@domoscargin domoscargin closed this Jan 3, 2025
@domoscargin domoscargin deleted the bk-refactoring branch January 14, 2025 21:08
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.

1 participant