doc: there is no case that is shown, so something was likely missing … #31762

tshepang · 2016-02-18T20:46:29Z

…from the change

rust-highfive · 2016-02-18T20:46:37Z

(rust_highfive has picked a reviewer for you, use r? to override)

steveklabnik · 2016-02-18T21:11:00Z

I am confused. It's showing what the value of this constant should be, given that we run the builders on this particular platform.

tshepang · 2016-02-18T21:15:53Z

Where is it shown that we are running on unix/linux?

steveklabnik · 2016-02-18T21:16:55Z

That's the idea with these. They're demonstrating the value this constant will have.

tshepang · 2016-02-18T21:20:14Z

But the constant depends on what platform was used to build the code, right?

steveklabnik · 2016-02-18T21:32:25Z

That's why it's "in this case". When these docs were built.

tshepang · 2016-02-18T21:34:58Z

I don't get it. There needs to be more explanation than just "in this case". More importantly, how is that info even important... why would we telling readers that the docs were built on unix/linux. I am also confused.

steveklabnik · 2016-02-18T21:38:42Z

Yeah, I didn't close it, because if its confusing, we should improve it.

These are string constants. This is intended to be showing the example value of said constant.

On Feb 18, 2016, 16:35 -0500, Tshepang Lekhonkhobenotifications@github.com, wrote:

I don't get it. There needs to be more explanation than just "in this case". More importantly, how is that info even important... why would we telling readers that the docs were built onunix/linux. I am also confused.

—
Reply to this email directly orview it on GitHub(#31762 (comment)).

MatejLach · 2016-02-18T21:50:07Z

@steveklabnik If I understand @tshepang correctly, this link shows it in context:
https://doc.rust-lang.org/std/env/consts/

I think what @tshepang is saying is that when you look at the docs, it doesn't make sense to say The family of the operating system. In this case, unix. - instead just say 'The family of the operating system.' or something like The family of the operating system. For example unix., because for a windows developer, it would be The family of the operating system. In this case, windows.

tshepang · 2016-02-18T21:52:22Z

@MatejLach understands me

steveklabnik · 2016-02-18T23:39:28Z

because for a windows developer, it would be The family of the operating system. In this case, windows.

Yes. This existing text was a compromise; there's no way to directly insert it, so I put in the example from the way that the docs will be displayed officially.

MatejLach · 2016-02-18T23:55:21Z

@steveklabnik Just to make sure, you're saying that you inserted the In this case, unix. because the docs on the website will be running on a Linux box?
If yes, then I think a wording like for example, unix is better than in this case, unix, since it takes some context inference to deduce what this case actually refers to.

steveklabnik · 2016-02-19T00:40:13Z

Because the builder that builds the docs is running on a Linux box.

On Feb 18, 2016, 18:55 -0500, Matej Ľachnotifications@github.com, wrote:

@steveklabnik(https://github.com/steveklabnik)Just to make sure, you're saying that you inserted theIn this case, unix.because the docs on the website will be running on a Linux box?

—
Reply to this email directly orview it on GitHub(#31762 (comment)).

frewsxcv · 2016-02-19T05:11:06Z

For what it's worth, I think the text

In this case, unix.

is confusing since it's not immediately obvious what "this" is referring to. In my opinion, I don't think the existence of that sentence adds much value.

mitaa · 2016-02-19T09:52:57Z

src/libstd/env.rs

@@ -618,7 +618,7 @@ pub mod consts {
    #[stable(feature = "env", since = "1.0.0")]
    pub const ARCH: &'static str = super::arch::ARCH;

-    /// The family of the operating system. In this case, `unix`.
+    /// The family of the operating system.


of the operating system in use

That would align this with the doc comment below, and then it's clear to which operating system this actually refers to.

Adding "in use" is an improvement, thanks.

mitaa · 2016-02-19T10:00:08Z

These items / doc-comments are never cfg'd out, so it seems like if one were to build windows documentation the docs would still show in this case unix instead of `in this case `windows

I think the listed possible values are clear enough.

steveklabnik · 2016-02-19T14:57:25Z

@mitaa yes, that's what I meant by

This existing text was a compromise;

tshepang · 2016-02-19T17:28:26Z

@steveklabnik have you seen this page https://doc.rust-lang.org/nightly/std/env/consts?

steveklabnik · 2016-02-19T17:45:03Z

Yes, I have.

steveklabnik · 2016-02-19T17:45:56Z

So, to be clear, what I want to see in an improvement here is to make sure that we have examples of what this constants look like. This was how I did it; other ways are welcome. I don't want to remove the example, though.

tshepang · 2016-02-22T16:27:26Z

There is no example in that page. What example are you referring to?

steveklabnik · 2016-02-22T16:50:23Z

These are constants. This is an example showing the value of said constants.

tshepang · 2016-02-22T18:13:58Z

You are talking about an example as if it exists...

I don't want to remove the example, though.

steveklabnik · 2016-02-22T19:21:45Z

The example is the text you are trying to remove in this PR.

MatejLach · 2016-02-22T19:21:56Z

@tshepang I think you're misunderstanding what @steveklabnik means by an example in this case.
I think what Steve means by an example in this case is the unix.
(Like, an example of an integer constant would be '1' and here a valid example is unix)
@steveklabnik That's why I proposed changing it from In this case, unix to for example, unix
The problem here is clarifying what this means, not the unix example itself.

ollie27 · 2016-03-02T16:25:01Z

There's a list of examples under the "Some possible values:" header. Is that not enough?

tshepang · 2016-03-03T08:01:37Z

They should be enough, but they don't render here. Maybe a bug in rustdoc?

mitaa · 2016-03-03T08:32:31Z

No, thats actually intentional. On module pages only short summaries are shown.
The full text is on the page of the const itself.
https://doc.rust-lang.org/nightly/std/env/consts/constant.ARCH.html

tshepang · 2016-03-03T08:53:27Z

Wow, wasn't even aware those are click-able. Well, that makes the case for this change even stronger, don't you think @steveklabnik?

tshepang · 2016-04-01T01:18:34Z

@steveklabnik ^

ranma42 · 2016-04-01T08:06:10Z

I agree with @steveklabnik that providing an example value in the short summary is convenient, but I also think that @MatejLach is right: the wording for example would be more clear, especially considered that the examples are not generated based on what environment is building the docs.

Also, ARCH has no example in the short summary, which feels like an inconsistency when reading the https://doc.rust-lang.org/std/env/consts/ page. Would it be ok to add an example for it?

steveklabnik · 2016-04-01T17:50:43Z

@tshepang sorry, I did not see this since @aturon was the reviewer, not me. Let's fix that:

r? @steveklabnik

the wording for example would be more clear,

Yes, this is what I'd like to see.

Would it be ok to add an example for it?

Yes, though I'm guessing the examples would move into the body, rather than be a summary.

tshepang · 2016-04-05T15:56:22Z

PR updated

steveklabnik · 2016-04-05T17:39:37Z

@bors: r+ rollup

bors · 2016-04-05T17:39:38Z

📌 Commit 8f463ea has been approved by steveklabnik

doc: there is no case that is shown, so something was likely missing … …from the change

Rollup of 12 pull requests - Successful merges: #31762, #32538, #32634, #32668, #32679, #32691, #32724, #32727, #32744, #32761, #32766, #32774 - Failed merges:

rust-highfive assigned aturon Feb 18, 2016

mitaa reviewed Feb 19, 2016
View reviewed changes

rust-highfive assigned steveklabnik and unassigned aturon Apr 1, 2016

doc: make env::consts summaries less confusing

8f463ea

Manishearth mentioned this pull request Apr 6, 2016

Rollup of 13 pull requests #32759

Closed

Manishearth added a commit to Manishearth/rust that referenced this pull request Apr 6, 2016

Rollup merge of rust-lang#31762 - tshepang:in-which-case, r=steveklabnik

6da3f85

doc: there is no case that is shown, so something was likely missing … …from the change

steveklabnik added a commit to steveklabnik/rust that referenced this pull request Apr 6, 2016

Rollup merge of rust-lang#31762 - tshepang:in-which-case, r=steveklabnik

c9168f7

doc: there is no case that is shown, so something was likely missing … …from the change

steveklabnik mentioned this pull request Apr 6, 2016

Rollup of 13 pull requests #32771

Closed

steveklabnik added a commit to steveklabnik/rust that referenced this pull request Apr 6, 2016

Rollup merge of rust-lang#31762 - tshepang:in-which-case, r=steveklabnik

858d37a

doc: there is no case that is shown, so something was likely missing … …from the change

steveklabnik mentioned this pull request Apr 6, 2016

Rollup of 12 pull requests #32772

Closed

steveklabnik added a commit to steveklabnik/rust that referenced this pull request Apr 6, 2016

Rollup merge of rust-lang#31762 - tshepang:in-which-case, r=steveklabnik

c411897

doc: there is no case that is shown, so something was likely missing … …from the change

steveklabnik mentioned this pull request Apr 6, 2016

Rollup of 12 pull requests #32778

Merged

bors added a commit that referenced this pull request Apr 6, 2016

Auto merge of #32778 - steveklabnik:rollup, r=steveklabnik

bf5da36

Rollup of 12 pull requests - Successful merges: #31762, #32538, #32634, #32668, #32679, #32691, #32724, #32727, #32744, #32761, #32766, #32774 - Failed merges:

bors merged commit 8f463ea into rust-lang:master Apr 6, 2016

tshepang deleted the in-which-case branch April 7, 2016 15:04

doc: there is no case that is shown, so something was likely missing … #31762

doc: there is no case that is shown, so something was likely missing … #31762

Conversation

tshepang commented Feb 18, 2016

rust-highfive commented Feb 18, 2016

steveklabnik commented Feb 18, 2016

tshepang commented Feb 18, 2016

steveklabnik commented Feb 18, 2016

tshepang commented Feb 18, 2016

steveklabnik commented Feb 18, 2016

tshepang commented Feb 18, 2016

steveklabnik commented Feb 18, 2016

MatejLach commented Feb 18, 2016

tshepang commented Feb 18, 2016

steveklabnik commented Feb 18, 2016

MatejLach commented Feb 18, 2016

steveklabnik commented Feb 19, 2016

frewsxcv commented Feb 19, 2016

mitaa Feb 19, 2016

Choose a reason for hiding this comment

tshepang Feb 19, 2016

Choose a reason for hiding this comment

mitaa commented Feb 19, 2016

steveklabnik commented Feb 19, 2016

tshepang commented Feb 19, 2016

steveklabnik commented Feb 19, 2016

steveklabnik commented Feb 19, 2016

tshepang commented Feb 22, 2016

steveklabnik commented Feb 22, 2016

tshepang commented Feb 22, 2016

steveklabnik commented Feb 22, 2016

MatejLach commented Feb 22, 2016

ollie27 commented Mar 2, 2016

tshepang commented Mar 3, 2016

mitaa commented Mar 3, 2016

tshepang commented Mar 3, 2016

tshepang commented Apr 1, 2016

ranma42 commented Apr 1, 2016

steveklabnik commented Apr 1, 2016

tshepang commented Apr 5, 2016

steveklabnik commented Apr 5, 2016

bors commented Apr 5, 2016