Skip to content

🌍 #376 Refactor - combine the sync and async check state machines#382

Merged
nth-commit merged 1 commit into
mainfrom
terra/async-refactor
May 25, 2023
Merged

🌍 #376 Refactor - combine the sync and async check state machines#382
nth-commit merged 1 commit into
mainfrom
terra/async-refactor

Conversation

@nth-commit

Copy link
Copy Markdown
Owner

This was a four month long battle. I picked it up many times throughout, each time being tempted by git branch -D terra/async-refactor.

Previously, one of the core pieces of infrastructure in GC was copy+pasted between Check/CheckAsync, it was a lot of lines of code to keep in sync. Even though CheckAsync is the primary usecase (it's the entrypoint used by GalaxyCheck.Xunit), I was not keen to drop the synchronous overload, as it would be a giant pain to litter the codebase with async/await.

This also means we can mostly forget about testing the async version of the code, as it barely deviates from the sync version at all, only at the entrypoint in CheckAsync.

@nth-commit
nth-commit force-pushed the terra/async-refactor branch 4 times, most recently from efa629b to 5a91f2a Compare May 25, 2023 22:56
This was a four month long battle. I picked it up many times throughout, each time being tempted by `git branch -D terra/async-refactor`.

Previously, one of the core pieces of infrastructure in GC was copy+pasted between Check/CheckAsync, it was a lot of lines of code to keep in sync. Even though CheckAsync is the primary usecase (it's the entrypoint used by GalaxyCheck.Xunit), I was not keen to drop the synchronous overload, as it would be a giant pain to litter the codebase with async/await.

This also means we can mostly forget about testing the async version of the code, as it barely deviates from the sync version at all, only at the entrypoint in `CheckAsync`.
@nth-commit
nth-commit force-pushed the terra/async-refactor branch from 5a91f2a to e5c0320 Compare May 25, 2023 23:01
@nth-commit
nth-commit merged commit 0168747 into main May 25, 2023
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