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

Add an injected assemblyValidation closure for ModuleAssembler #89

Merged
merged 1 commit into from
Oct 25, 2023

Conversation

bradfol
Copy link
Collaborator

@bradfol bradfol commented Oct 25, 2023

This will allow ScopedModuleAssembler to inject its validation rather than performing it at the very end.

@bradfol bradfol requested a review from skorulis-ap October 25, 2023 20:42
@bradfol bradfol force-pushed the bradfol/assembly-validation branch 2 times, most recently from b0649f4 to 45c3487 Compare October 25, 2023 20:48
// if the root assembly is targeting an incorrect resolver.
if let assemblyValidation {
do {
try assemblyValidation(from)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Should this check resolved instead of from as that is the type that will actually be used.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Ah good catch I'll update

Copy link
Collaborator

@skorulis-ap skorulis-ap left a comment

Choose a reason for hiding this comment

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

Looks good

This will allow ScopedModuleAssembler to inject its validation rather than performing it at the very end.
@bradfol bradfol force-pushed the bradfol/assembly-validation branch from 45c3487 to 59c0d0d Compare October 25, 2023 22:06
@bradfol bradfol enabled auto-merge October 25, 2023 22:07
@bradfol bradfol merged commit 3ffd14c into main Oct 25, 2023
1 check passed
@bradfol bradfol deleted the bradfol/assembly-validation branch October 25, 2023 22: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.

2 participants