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

grammar: fix #33008

Merged
merged 3 commits into from
Apr 19, 2016
Merged

grammar: fix #33008

merged 3 commits into from
Apr 19, 2016

Conversation

sanmai-NL
Copy link

Reading this, one item stood out a bit. Small improvements here.

  1. ‘Compile-time’ is not a noun, ‘compilation time’ was meant;
  2. Mathematical formulas are best not rendered as code;
  3. Use the same tense as in other items.

Reading this, one item stood out a bit. Small improvements here.

. ‘Compile-time’ is not a noun, ‘compilation time’ was meant;
. Mathematical formulas are best not rendered as code;
. Use the same tense as in other items.
@rust-highfive
Copy link
Collaborator

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @brson (or someone else) soon.

If any changes to this PR are deemed necessary, please add them as extra commits. This ensures that the reviewer can see what has changed since they last reviewed the code. Due to the way GitHub handles out-of-date commits, this should also make it reasonably obvious what issues have or haven't been addressed. Large or tricky changes may require several passes of review and changes.

Please see the contribution instructions for more information.

@brson
Copy link
Contributor

brson commented Apr 15, 2016

r? @steveklabnik not sure about the math conventions

@rust-highfive rust-highfive assigned steveklabnik and unassigned brson Apr 15, 2016
@steveklabnik
Copy link
Member

I am generally pro this PR, but we don't have any explicit convention regarding the math, and I think it would generally be displayed this way elsewhere. What's the specific objection here?

Sander Maijers added 2 commits April 16, 2016 10:55
@sanmai-NL
Copy link
Author

sanmai-NL commented Apr 16, 2016

I've submitted this PR for mainly just the typo in ‘equivalence’. The math style thing was something extra, should have been split off into a different commit.

The big-O expression looks more conventional (i.e., MathJax style) when rendered in text font, even if sans serif, than when rendered in code font (i.e., monospace). The philosophy: it's better to do nothing (format math as text) than to do something that would be even more wrong (tag math as code).

GitHub Flavored Markdown (GFM) does not support displaying math, it seems. Neither does GitHub render math when using e.g. the more complete markup language AsciiDoc. For big-O formulas, normal text formatting may suffice though. I've amended the PR, and I think it looks fine now.

So, maximally exploiting GFM's standard text formatting functionality plus Unicode symbols could be taken as convention for math authoring for Rust. Alternatively, documentation could be partially migrated to Asciidoctor, as the OCaml team are doing recently. The value of that beyond math support would be a nicely formatted changelog at least outside GitHub (plus ToC, snippet inclusion, etc.).

@GuillaumeGomez
Copy link
Member

I'm on @sanmai-NL side. This look better like this. However, do we have an O notation somewhere else?

@steveklabnik
Copy link
Member

We do use big O notation in a a few places, and I know at least every time I've written it, it's been in code.

But, given that we have no strong convention, I will just merge this, and if we decide on an official convention we'll have to adjust them all anyway.

@bors: r+ rollup

@bors
Copy link
Contributor

bors commented Apr 18, 2016

📌 Commit 8ff34c7 has been approved by steveklabnik

steveklabnik added a commit to steveklabnik/rust that referenced this pull request Apr 18, 2016
grammar: fix

Reading this, one item stood out a bit. Small improvements here.

1. ‘Compile-time’ is not a noun, ‘compilation time’ was meant;
1. Mathematical formulas are best not rendered as code;
1. Use the same tense as in other items.
bors added a commit that referenced this pull request Apr 18, 2016
Rollup of 6 pull requests

- Successful merges: #32558, #32906, #33007, #33008, #33035, #33058
- Failed merges: #32912
@bors bors merged commit 8ff34c7 into rust-lang:master Apr 19, 2016
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.

6 participants