Skip to content

Replace usage of println! with logger macros#2020

Merged
alamb merged 1 commit intoapache:masterfrom
silence-coding:master
Mar 20, 2022
Merged

Replace usage of println! with logger macros#2020
alamb merged 1 commit intoapache:masterfrom
silence-coding:master

Conversation

@silence-coding
Copy link
Copy Markdown
Contributor

The println macro used in the code is changed to the info macro.

@silence-coding
Copy link
Copy Markdown
Contributor Author

The reason for using info is that I/O exceptions often occur. Errors caused by error files are very common. Therefore, warn is not necessary.

Copy link
Copy Markdown
Contributor

@alamb alamb left a comment

Choose a reason for hiding this comment

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

Thank you @silence-coding - I agree println! is not ideal and logging is much better

I think this PR is an improvement -- maybe warn! or error! would be better still

partition_col_proj,
) {
println!(
info!(
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

What would you think about using warn! or error!here instead ofinfo!`?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

+1, warn! maybe better.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

What would you think about using warn! or error!here instead ofinfo!`?

It's all acceptable to me except println

@jimexist jimexist changed the title I think using info in formal code is better than using println. Replace usage of println! with logger macros Mar 16, 2022
@alamb
Copy link
Copy Markdown
Contributor

alamb commented Mar 20, 2022

I think info! is better than println! -- and we can adjust the level down in the future if that is better still. Thanks again @silence-coding

@alamb alamb merged commit 7ed3be6 into apache:master Mar 20, 2022
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.

3 participants