convert the PackageLoading module to the new diagnostics apis #3754
Conversation
motivation: adapt new diagnostics apis changes: refactor the PackageLoading module to use new diagnostics api
tomerd
requested review from
abertelrud,
friedbunny and
neonichu
as code owners
|
@swift-ci please smoke test |
tomerd
added
the
ready
| // The following rules are documented at https://github.com/apple/swift-package-manager/blob/master/Documentation/Usage.md#creating-c-language-targets. To avoid breaking existing packages, do not change the semantics here without making any change conditional on the tools version of the package that defines the target. | ||
|
|
||
| let diagnosticsEmitter = observabilityScope.makeDiagnosticsEmitter { |
There was a problem hiding this comment.
Is this part about creating a diagnostics emitter temporary until the old diagnostics engine has been removed, or will every client function create its own emitter in the future as well?
There was a problem hiding this comment.
in most cases, code would emit directly from the ObservabilityScope, however, there are cases where you may want to customize metadata further without nesting another scope, which you can do in one of two ways: either create a DiagnosticsEmitter with additional metadata , or specific the additional metadata on the emit() call itself. in this case we choose the former since the code emit in many places so its more efficient to hold a emitter with metadata set.
There was a problem hiding this comment.
Makes sense. It will be interesting to see how this can be simplified if we have task locals or similar. It seems compelling to want to wrap a closure and add metadata to any diagnostics it emits without quite so much ceremony.
There was a problem hiding this comment.
agreed. when we migrate the code to 5.5/5.6 we would be able to take advantage of task locals
There was a problem hiding this comment.
Nice! Right, it wasn't so much a commentary on this PR as thinking ahead. This looks good to me.
motivation: adapt new diagnostics apis
changes: refactor the PackageLoading module to use new diagnostics api