-
Notifications
You must be signed in to change notification settings - Fork 204
Verify module interfaces generated from emit-module jobs #615
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
Conversation
@swift-ci test |
The Verify job was previously only added after merge-module jobs.
b510621
to
331d2fe
Compare
@swift-ci test |
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.
A couple of nits, but overall looks good to me!
@@ -253,9 +257,9 @@ extension Driver { | |||
addJob: (Job) -> Void, | |||
addJobOutputs: ([TypedVirtualPath]) -> Void, | |||
emitModuleTrace: Bool | |||
) throws { | |||
) throws -> Job? { |
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.
Just because the name states the plural Jobs
, and we may be adding multiple (with backend jobs), it's worth a comment to explain exactly which job this function is going to return.
addJobOutputs: addJobOutputs, | ||
emitModuleTrace: loadedModuleTracePath != nil) | ||
emitModuleTrace: loadedModuleTracePath != nil) { | ||
try addVerifyJobs(mergeJob: compileJob, addJob: addJobAfterCompiles) |
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.
Nit: it may be time to re-name this argument name from mergeJob
to something that better reflects that this can be any interface-producing job.
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.
Done!
@swift-ci please test |
@swift-ci please test Linux platform |
Verify interfaces generated by single compile jobs and emit-module jobs. The verify job was previously only added after merge-module jobs which is more prone to generate broken swift interfaces.