Skip to content

Load manifests asynchronously #3894

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

Merged
merged 11 commits into from
Dec 13, 2021
Merged

Conversation

pcbeard
Copy link
Contributor

@pcbeard pcbeard commented Dec 1, 2021

See #3872 for full description.

This adds one more commit, to remove a throws from the evaluateManifest() method.

@pcbeard
Copy link
Contributor Author

pcbeard commented Dec 1, 2021

@swift-ci please smoke test

@tomerd
Copy link
Contributor

tomerd commented Dec 1, 2021

@pcbeard @abertelrud how about we let all the other changes time (couple of days) to propagate through the system then merge this?

@pcbeard
Copy link
Contributor Author

pcbeard commented Dec 2, 2021

@tomerd Sounds like a good plan.

@pcbeard pcbeard force-pushed the AsyncManifestLoading2 branch from 5f6a6df to 81c1d6e Compare December 2, 2021 21:03
@tomerd
Copy link
Contributor

tomerd commented Dec 2, 2021

@swift-ci please smoke test

@tomerd tomerd added the ready Author believes the PR is ready to be merged & any feedback has been addressed label Dec 2, 2021
DelayableAction represents an action on a target that can be deferred
until a more appropriate time. In this case, we use defer to cleanup
objects in the case of early return error handling, but we need to
delay this action when passing the target for use into an async
completion block.
The manifest parser uses SourceControl.GitShellHelper synchronously,
which would cause a hang if using the default queue for completions.
This is required for unit tests to pass.
@pcbeard pcbeard force-pushed the AsyncManifestLoading2 branch from 81c1d6e to 921f9c0 Compare December 2, 2021 22:47
@pcbeard
Copy link
Contributor Author

pcbeard commented Dec 2, 2021

@swift-ci please smoke test

@tomerd tomerd merged commit 4293786 into swiftlang:main Dec 13, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ready Author believes the PR is ready to be merged & any feedback has been addressed
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants