Skip to content
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

builder-tool: support -alwaysShowBuildLog option for local builds. #642

Merged
merged 4 commits into from
Sep 13, 2019
Merged

builder-tool: support -alwaysShowBuildLog option for local builds. #642

merged 4 commits into from
Sep 13, 2019

Conversation

rgooch
Copy link
Contributor

@rgooch rgooch commented Sep 10, 2019

This enables real-time display of build logs in builder-tool when building locally.
Minor fix for built-in help.
Fix a panic when building a tree locally, which never has a filter (bug introduced in a previous commit).

rootDir, err := builder.BuildTreeFromManifest(srpcClient, args[0], buildLog,
logger)
logWriter := &logWriterType{}
if *alwaysShowBuildLog {
Copy link
Contributor

Choose a reason for hiding this comment

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

why dont we push these diffs into the implementaiton of the &logWriterType{} ? (maybe ioWriteCloser?)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

So, you're suggesting writing the Start... message on the first write and then add a Close() method which writes the End message and writes the file?

I could do that. I'd prefer to do that as a separate PR, as I've got a lot of pending commits. Also, I might want to make this into a library package if I can think of a nice abstraction.

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes.... (I will take the same response for my blank lines :))

@cviecco cviecco merged commit 5c63b23 into Symantec:master Sep 13, 2019
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.

None yet

2 participants