Skip to content

[Build System: build-script] Remove the diagnostics module from swift_build_support. #29603

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
Feb 3, 2020
Merged

[Build System: build-script] Remove the diagnostics module from swift_build_support. #29603

merged 1 commit into from
Feb 3, 2020

Conversation

Rostepher
Copy link
Contributor

The diagnostics module is a rather poor replacement for actual logging. Since it's only used in a few places I've either inlined the calls or added equivalent helper functions for printing notes and exiting with a fatal error.

In the future we should adopt the standard Python logging module which is thread safe.

@Rostepher
Copy link
Contributor Author

@swift-ci please test

@compnerd
Copy link
Member

compnerd commented Feb 3, 2020

A strong +1 on the use of logging instead of this.

compnerd
compnerd approved these changes Feb 3, 2020
View reviewed changes
Copy link
Member

@compnerd compnerd left a comment

Choose a reason for hiding this comment

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

Id prefer using logging, but I suppose doing that in a separate change isnt too terrible

@Rostepher
Copy link
Contributor Author

Id prefer using logging, but I suppose doing that in a separate change isnt too terrible

We can transition to logging in a separate PR, but that's definitely where we should be going.

@Rostepher Rostepher merged commit e8f2119 into swiftlang:master Feb 3, 2020
@Rostepher Rostepher deleted the remove-diagnostics-module branch February 3, 2020 00:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@compnerd compnerd compnerd approved these changes

@drodriguez drodriguez Awaiting requested review from drodriguez

@gottesmm gottesmm Awaiting requested review from gottesmm

@edymtt edymtt Awaiting requested review from edymtt

@shahmishal shahmishal Awaiting requested review from shahmishal

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@Rostepher @compnerd