Skip to content

Conversation

@franklinsch
Copy link
Contributor

What's in this pull request?

A successMessage parameter is added to runLite, which provides the ability to customize the message shown if all the tests pass.

The default message "All tests pass! 🎉" displayed if no success message is specified.

The message is shown in bold green.

What's worth discussing about this pull request?

Should the user be able to customize the color and boldness as well?

What downsides are there to merging this pull request?

🤷🏻‍♂️

@franklinsch
Copy link
Contributor Author

Looks like https://swiftenv.fuller.li/install.sh is down.

pathExtensions: Set<String>, testLinePrefix: String,
parallelismLevel: ParallelismLevel) throws {
parallelismLevel: ParallelismLevel,
successMessage: String = "All tests passed! 🎉") throws {

Choose a reason for hiding this comment

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

This is nice!
I think the default here can be removed, since it’s passed in from Run.swift line 39

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good idea. I've removed the default in TestRunner.

If all the tests of a lite instance pass, the string passed as the
`successMessage` parameter of `runLite` will be displayed.
A default message is displayed if no success message is specified.
@CodaFi
Copy link
Member

CodaFi commented Apr 20, 2018

@franklinsch This patch LGTM. Despite the missing script this passes on macOS and I have no doubt it will pass on Linux as well.

Will give this patch until midnight my time and merge then regardless of CI status

@franklinsch
Copy link
Contributor Author

@CodaFi Looks like the swiftenv script is working again.

@harlanhaskins
Copy link
Member

⛵ thank you @franklinsch!!

@harlanhaskins harlanhaskins merged commit dc58d77 into llvm-swift:master Apr 22, 2018
@franklinsch
Copy link
Contributor Author

@harlanhaskins Could we make a release for this patch? So that I can import it using SwiftPM

@harlanhaskins
Copy link
Member

Tagged 0.0.10!

@franklinsch
Copy link
Contributor Author

That was fast, thanks!

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.

4 participants