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

Ets and Dets code should not be printing to stdout #9232

Merged
merged 2 commits into from
Jan 13, 2025

Conversation

richcarl
Copy link
Contributor

@richcarl richcarl commented Dec 21, 2024

Some quite old code in Ets and Dets would print messages to stdout when repairing tables or when fun2ms failed.

Copy link
Contributor

github-actions bot commented Dec 21, 2024

CT Test Results

    2 files     96 suites   1h 7m 33s ⏱️
2 173 tests 2 125 ✅ 48 💤 0 ❌
2 536 runs  2 486 ✅ 50 💤 0 ❌

Results for commit 3bad824.

♻️ This comment has been updated with latest results.

To speed up review, make sure that you have read Contributing to Erlang/OTP and that all checks pass.

See the TESTING and DEVELOPMENT HowTo guides for details about how to run test locally.

Artifacts

// Erlang/OTP Github Action Bot

@IngelaAndin IngelaAndin added team:VM Assigned to OTP team VM team:PS Assigned to OTP team PS labels Jan 3, 2025
Copy link
Contributor

@IngelaAndin IngelaAndin left a comment

Choose a reason for hiding this comment

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

I think you should use logger instead of calling legacy error_logger functions.

@richcarl
Copy link
Contributor Author

richcarl commented Jan 7, 2025

I think you should use logger instead of calling legacy error_logger functions.

That would introduce a mix of old error_logger calls and new logger calls. I think it is better to follow the existing code style and then replace all of the calls in a separate update.

@IngelaAndin IngelaAndin self-assigned this Jan 7, 2025
@IngelaAndin
Copy link
Contributor

@richcarl well ok we will accept that, as at least it is an improvement over using io:format. However the PS-team does not feel
that it is granted to remove the debug printouts that have no affect unless you uncomment some lines and recompile. Although this is not a coding style that we prefer, this code is old and may still have a educational purpose for someone that needs to maintain it. And we would rather keep it for now and maybe it will be refactored sometime in the future.

@richcarl
Copy link
Contributor Author

Fair enough, I dropped the macro removal. IMO though, the only place for that sort of code is in the version control history.

@IngelaAndin IngelaAndin added the testing currently being tested, tag is used by OTP internal CI label Jan 10, 2025
@IngelaAndin IngelaAndin self-requested a review January 10, 2025 18:55
@IngelaAndin IngelaAndin merged commit 7e07bb8 into erlang:master Jan 13, 2025
24 checks passed
@richcarl richcarl deleted the ets-no-print branch January 13, 2025 15:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
team:PS Assigned to OTP team PS team:VM Assigned to OTP team VM testing currently being tested, tag is used by OTP internal CI
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants