-
Notifications
You must be signed in to change notification settings - Fork 551
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
ci: Support configuring the logging level for the splitter package #2579
ci: Support configuring the logging level for the splitter package #2579
Conversation
Update the split package and add support for configuring the logging level to avoid CI interpretting info-level logging as error messages. Signed-off-by: timflannagan <[email protected]>
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: timflannagan 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 |
@@ -103,14 +112,14 @@ func findDescribes(dir string) ([]string, error) { | |||
} | |||
specNames := topDescribeRE.FindAllSubmatch(b, -1) | |||
if len(specNames) == 0 { | |||
log.Printf("%s: found no top level describes, skipping", match) | |||
logger.Warnf("%s: found no top level describes, skipping", match) |
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.
Another implementation: we could also just remove logging from this package entirely. It's nice for debugging this package while developing locally, but provides little value when running in CI environments.
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.
If it's not getting in the way, I'd say leave it. Logging is helpful during development. Though a debugger is also nice...
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.
/lgtm
Update the split package and add support for configuring the logging
level to avoid CI interpretting info-level logging as error messages.
Signed-off-by: timflannagan [email protected]
Description of the change:
Add support for configuring the logging level for the e2e splitter package, and default to the error level.
Motivation for the change:
This is to workaround CI interpreting the previous log.Print* calls as error-level messages. A concrete example can be found in this action run: https://github.com/operator-framework/operator-lifecycle-manager/actions/runs/1724759061 where "found no top level describes, skipping" has a red market above it.
Reviewer Checklist
/doc