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

Requirements checker now logs to stderr #679

Merged
merged 3 commits into from
Sep 25, 2022

Conversation

arnegroskurth
Copy link
Contributor

Fixes #678

Copy link
Member

@theofidry theofidry left a comment

Choose a reason for hiding this comment

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

Thanks for the PR!

A few nitpicks but looks good to me! Since there is so much changes for the next release I'd more confident to tag a new minor version.

requirement-checker/tests/bootstrap.php Outdated Show resolved Hide resolved
requirement-checker/src/Printer.php Outdated Show resolved Hide resolved
requirement-checker/src/Printer.php Outdated Show resolved Hide resolved
doc/requirement-checker.md Outdated Show resolved Hide resolved
UPGRADE.md Outdated Show resolved Hide resolved
@theofidry
Copy link
Member

🤔 not sure why the End-to-End Tests / e2e-Tests: e2e_check_requirements build is failing. The build is passing on master so it should be related, but tee should pick up the stderr

@arnegroskurth
Copy link
Contributor Author

thinking not sure why the End-to-End Tests / e2e-Tests: e2e_check_requirements build is failing. The build is passing on master so it should be related, but tee should pick up the stderr

I'm not really able to interpret the output but the latter failing test-case seems to complain about failed writes due to potentially full disk: https://github.com/box-project/box/runs/7039442617?check_suite_focus=true#step:6:2056

And I don't think that teeforwards stderr (Try: sh -c "echo foo > /dev/stderr" | tee file.ext) - where is that used? Just setting BOX_REQUIREMENTS_CHECKER_LOG_TO_STDOUT=1 for all tests might be the easiest way out here.

@theofidry
Copy link
Member

I'm not really able to interpret the output but the latter failing test-case seems to complain about failed writes due to potentially full disk

Yes you can ignore this one I need to check how to fix it.

where is that used?

In the Makefile. You should be able to run the tests with make e2e_check_requirements

UPGRADE.md Outdated Show resolved Hide resolved
@theofidry theofidry merged commit 676e7ee into box-project:master Sep 25, 2022
@theofidry
Copy link
Member

Thank a lot @arnegroskurth

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.

Requirements Checker outputs on stdout instead of stderr
2 participants