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

Revert "Better warning for --build-loglevel with binary builds" #19320

Merged
merged 1 commit into from
Apr 12, 2018

Conversation

bparees
Copy link
Contributor

@bparees bparees commented Apr 11, 2018

@openshift-ci-robot openshift-ci-robot added size/M Denotes a PR that changes 30-99 lines, ignoring generated files. approved Indicates a PR has been approved by an approver from all required OWNERS files. labels Apr 11, 2018
@bparees
Copy link
Contributor Author

bparees commented Apr 11, 2018

/test extended_builds

@bparees
Copy link
Contributor Author

bparees commented Apr 11, 2018

@nak3 please create a new PR that fixes the message but retains its status as a warning, not an error.

@bparees
Copy link
Contributor Author

bparees commented Apr 11, 2018

@coreydaley or @gabemontero please give this a quick signoff.

@gabemontero
Copy link
Contributor

/lgtm

@openshift-ci-robot openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Apr 11, 2018
@bparees
Copy link
Contributor Author

bparees commented Apr 11, 2018

flake #19282
/retest

@coreydaley
Copy link
Member

/lgtm

@openshift-ci-robot
Copy link

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: bparees, coreydaley, gabemontero

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@liggitt
Copy link
Contributor

liggitt commented Apr 11, 2018

flake #19282

fixed in #19321

@bparees
Copy link
Contributor Author

bparees commented Apr 11, 2018

/retest

@nak3
Copy link
Contributor

nak3 commented Apr 12, 2018

@bparees Thank you & I'm sorry for my mistake. I will send the PR after this revert was done.

But please allow me to learn from this my mistake. The error is from this code:

br, err := exutil.StartBuildAndWait(oc, "statusfail-assemblescript", fmt.Sprintf("--from-dir=%s", binaryBuildDir), "--build-loglevel=5")
o.Expect(err).NotTo(o.HaveOccurred())

, and it runs binary build with --build-loglevel=5.

Removing --build-loglevel from this binary build code (statusfail-assemblescript) was not an option to fix the broken test? I understood warning->error broke the test, but I would like to learn why this code needs to have --build-loglevel=5 with binary build, (which is invalid option actually).

@openshift-merge-robot openshift-merge-robot merged commit 1705a22 into master Apr 12, 2018
@nak3
Copy link
Contributor

nak3 commented Apr 12, 2018

I'm sorry, I found the comment in #19120 (comment) Please disregard my question above.

@bparees
Copy link
Contributor Author

bparees commented Apr 12, 2018

I would like to learn why this code needs to have --build-loglevel=5 with binary build, (which is invalid option actually).

the test doesn't need it and it should probably be removed from the test, but in this case it was actually probably a good thing it had it so we caught this before it broke a customer's script/pipeline.

@nak3
Copy link
Contributor

nak3 commented Apr 12, 2018

@bparees Thank you so much. I understand and learned a lot.

@stevekuznetsov stevekuznetsov deleted the revert-19120-warn-loglevel branch July 16, 2018 23:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. lgtm Indicates that a PR is ready to be merged. queue/critical-fix size/M Denotes a PR that changes 30-99 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

extended_builds: failute_status "Build path output did not match expected format 'build/name' : \"\""
7 participants