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

Make sure we have pulled before reading LyraConfig #62

Merged
merged 9 commits into from
Mar 29, 2024

Conversation

WULCAN
Copy link
Collaborator

@WULCAN WULCAN commented Mar 17, 2024

Move all initializing git pull into RepoGit and make RepoGit shared for each unique local git clone.

As the repository specific configuration file is potentially stale, we need to do this initializing pull before any read of that configuration file. To ensure that, I replace all direct reads with a read through RepoGit. Since the factory always pulls, we know the config will be fresh after a Lyra server start.

Note that the route for creating pull-requests can now theoretically pull twice, if RepoGit was not initialzied first. That is however extremely unlikely as button the create pull requests is on a page that uses RepoGit to load its data. Anyway, I beleive we will resolve this when we implement resyncing from the remotes.

@WULCAN WULCAN force-pushed the undocuented/centralize_initial_pull branch from 8c3eb3e to 47d9ea2 Compare March 17, 2024 11:22
Move all initializing git pull into RepoGit and make RepoGit shared for
each unique local git clone.

As the repository specific configuration file is potentially stale,
we need to do this initializing pull before any read of that
configuration file. To ensure that, I replace all direct reads with a
read through RepoGit. Since the factory always pulls, we know the config
will be fresh after a Lyra server start.

Note that the route for creating pull-requests can now theoretically
pull twice, if RepoGit was not initialzied first. That is however
extremely unlikely as button the create pull requests is on a page that
uses RepoGit to load its data. Anyway, I beleive we will resolve this
when we implement resyncing from the remotes.
@WULCAN WULCAN force-pushed the undocuented/centralize_initial_pull branch from 47d9ea2 to e0c168f Compare March 17, 2024 11:28
Copy link
Collaborator

@amerharb amerharb left a comment

Choose a reason for hiding this comment

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

This is really good refactoring, I made some comments also a small PR if you agree about the changes

webapp/README.md Show resolved Hide resolved
webapp/src/RepoGit.ts Outdated Show resolved Hide resolved
webapp/src/RepoGit.ts Outdated Show resolved Hide resolved
webapp/src/app/api/pull-request/[projectName]/route.ts Outdated Show resolved Hide resolved
amerharb and others added 3 commits March 26, 2024 00:32
@richardolsson
Copy link
Member

Having discussed this with @WULCAN, we consider this PR approved since the changes that @amerharb requested have been added through the merging of #63.

@richardolsson richardolsson merged commit a6bb5e1 into main Mar 29, 2024
1 check passed
@richardolsson richardolsson deleted the undocuented/centralize_initial_pull branch March 29, 2024 11:17
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.

3 participants