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

Lint usage of Debug-based formatting #637

Merged
merged 4 commits into from
Feb 12, 2016
Merged

Conversation

mcarton
Copy link
Member

@mcarton mcarton commented Feb 7, 2016

Fix #634.
Ref #461.
Ref #442.

@mcarton
Copy link
Member Author

mcarton commented Feb 11, 2016

Any comment on that one?

@llogiq
Copy link
Contributor

llogiq commented Feb 12, 2016

Too much to look at from mobile; it'll have to wait until the afternoon.

}
}

impl fmt::Display for Constant {
Copy link
Contributor

Choose a reason for hiding this comment

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

Is there a specific reason for removing this (apart from it using Debug formatting to write out Strs)?

Copy link
Member Author

Choose a reason for hiding this comment

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

It was used only once, only to print integers. After I refactored the loop lint to use const_eval it wasn’t used anymore and seemed of little use, if the user wrote 6*7 in an expression, they must have a reason not to have written 42, a snippet should always be used to print the original expression.

@llogiq
Copy link
Contributor

llogiq commented Feb 12, 2016

Apart from one question, looks good to me.

llogiq added a commit that referenced this pull request Feb 12, 2016
Lint usage of `Debug`-based formatting
@llogiq llogiq merged commit 9ee4626 into rust-lang:master Feb 12, 2016
@llogiq
Copy link
Contributor

llogiq commented Feb 12, 2016

Thanks!

@mcarton mcarton deleted the debug branch February 12, 2016 11:25
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.

2 participants