-
Notifications
You must be signed in to change notification settings - Fork 0
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
Add seed data #27
Merged
Merged
Add seed data #27
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
1399faa
to
82ec11e
Compare
5bbac31
to
9091893
Compare
danlivings-dxw
approved these changes
Sep 11, 2024
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.
This implements issue #26
Looks good to me, I just have a minor comment.
I think we might want to look into running the seed script on a schedule when this is deployed to avoid having to do it manually, but shouldn't be a blocker for a first iteration.
737e602
to
5211e27
Compare
This function won't be used to map repos from the Github API to the UI anymore but from our stored repos to the UI repos. This means we can just convert the date from ISO8601 to a JS Date object. We retain the total number of repos as this may come in handy later.
This is called pre-mapping so we use the casing from Github's API
This script pulls in all repos for the org where Towtruck is installed. It pulls the relevant properties from the repo data and pushes this info to an array. We ignore any archived repos which reduces the number of repos by >50%. We then write this repo data to a JSON file to be used later. I couldn't work out a way to unit test the function that didn't result in tests that were harder to read than the function itself. The mocking in Node's built in test runner is still experimental so we may need to introduce a standalone test runner (Vitest, Jest) to be able to mock properly.
We want to be able to display the name of the organisation too
This simply reads from the data added by the function in the last commit.
Previously we were fetching all the repos on each request to the server by iterating through each installation of the app and every repo of that installation. Now we just read from the seeded JSON instead.
The app will no longer work unless the seed script is run. We don't always want to seed this data as we hit the prod GH API. For now the seeding can be run by passing a flag to the script/bootstrap command.
We have some new data it is useful to show
5211e27
to
7e3463a
Compare
richpjames
pushed a commit
that referenced
this pull request
Sep 12, 2024
I broke this with #27. I switched from passing repos as a second parameter to passing it as part of the object in the first parameter in the consuming function but not in the function signature itself. This format follows the signature the Octokit builtin functions use so I think it makes sense to align our approach with theirs.
richpjames
pushed a commit
that referenced
this pull request
Sep 12, 2024
I broke this with #27. I switched from passing repos as a second parameter to passing it as part of the object in the first parameter in the consuming function but not in the function signature itself. This format follows the signature the Octokit builtin functions use so I think it makes sense to align our approach with theirs.
richpjames
pushed a commit
that referenced
this pull request
Sep 12, 2024
I broke this with #27. I switched from passing repos as a second parameter to passing it as part of the object in the first parameter in the consuming function but not in the function signature itself. This format follows the signature the Octokit builtin functions use so I think it makes sense to align our approach with theirs.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Previously we were hitting the Github API to fetch the repos every time the server received a request.
This next iteration adds a seed step where the repo data is fetched and saved to JSON per #21. Every request the server receives then reads from this data and renders it to this page. We also add some more data to the table UI. It is a naive approach with lots of room for improvement but I think it's enough for now.
Future improvements
NB to run the app locally you will now either need to run
script/seed
orscript/boostrap -seed
.