-
Notifications
You must be signed in to change notification settings - Fork 61
Check output tool and install Xcpretty if it is missing #115
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
Squashed commits: [31f5a78] clean
| - path::./: | ||
| inputs: | ||
| - output_tool: xcodebuild | ||
| - output_tool: $OUTPUT_TOOL |
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.
OUTPUT_TOOL is set to xcpretty (which is the default value for this input) please wire in test for xcodebuild output tool
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.
The test_objc workflow using the xcodebuild as an output tool.
main.go
Outdated
| } | ||
| } | ||
|
|
||
| if err == nil { |
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.
why is it needed?
main.go
Outdated
| log.Printf("Switching to xcodebuild for output tool") | ||
| outputTool = "xcodebuild" | ||
| } | ||
| log.Printf("- xcprettyVersion: %s", xcprettyVersion.String()) |
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.
do this after you made sure that xcpretty is available or switched to xcodebuild (if outputtool == xcpretty {...)
step.yml
Outdated
| - xcodebuild | ||
| is_required: true | ||
| is_expand: false | ||
| is_expand: true |
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.
this is the default value, pls remove
| } | ||
|
|
||
| // IsToolInstalled ... | ||
| func IsToolInstalled(name string) bool { |
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.
was it only used for xcpretty?
utils/utils.go
Outdated
| cmd := command.New("which", name) | ||
| out, err := cmd.RunAndReturnTrimmedCombinedOutput() | ||
| return err == nil && out != "" | ||
| func IsToolInstalled(name, version string) (bool, error) { |
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.
do we need this at all? it just wraps a single line of command
utils/utils.go
Outdated
| func InstallXcpretty() error { | ||
| cmds, err := rubycommand.GemInstall("xcpretty", "") | ||
| if err != nil { | ||
| return fmt.Errorf("failed to create command model, error: %s", err) |
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.
failed to create %s install command, xcpretty
utils/utils.go
Outdated
| } | ||
|
|
||
| for _, cmd := range cmds { | ||
| log.Donef("$ %s", cmd.PrintableCommandArgs()) |
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.
do not use green coloring, this is just a small utility, but not a big milestone in the step, like other Done logs
godrei
left a comment
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.
pls have a look at my comments
Squashed commits: [014f40e] PR clean - fix
db606f8 to
167c8a0
Compare
No description provided.