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

revised a jemalloc statement in how-to-build-and-run.md #229

Merged
merged 2 commits into from
Nov 7, 2018

Conversation

kenta7777
Copy link
Contributor

I revised a jemalloc statement concerned with creating config.toml.

@kenta7777
Copy link
Contributor Author

Fixes #227.

@mark-i-m
Copy link
Member

mark-i-m commented Nov 6, 2018

Thanks! r=me when Travis passes

@ehuss
Copy link
Contributor

ehuss commented Nov 6, 2018

The default is off, right? Is there any reason to even mention jemalloc?

@mark-i-m
Copy link
Member

mark-i-m commented Nov 6, 2018

It's a fair question. This was originally included by Niko, so my inclination is to keep it unless you feel strongly about it...

@ehuss
Copy link
Contributor

ehuss commented Nov 6, 2018

It's a fair question. This was originally included by Niko, so my inclination is to keep it unless you feel strongly about it...

It was included because the default was "on", and turning it off can help with development. Now that it is off by default, I don't see a reason to mention it (there are many other settings in the config that aren't mentioned because the defaults are fine). Also, this PR removes the original comment (which was unique to this guide) that explains why you would turn it off, and replaces it with a comment that is confusing to me (the way I read it it, it implies that it is adding jemalloc) — the other comments explain what it does, this one explains what it doesn't do.

@mark-i-m
Copy link
Member

mark-i-m commented Nov 6, 2018

I see what you mean. So @kenta7777 could you actually just remove these lines altogether?

@kenta7777
Copy link
Contributor Author

@mark-i-m Certainly. I'll remove these lines.

Copy link
Member

@mark-i-m mark-i-m left a comment

Choose a reason for hiding this comment

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

Thanks!

@mark-i-m mark-i-m merged commit dc9ceca into rust-lang:master Nov 7, 2018
@kenta7777 kenta7777 deleted the kenta7777#227 branch November 7, 2018 15:00
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