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

Set --extend-css stable #41700

Merged
merged 2 commits into from
May 25, 2017
Merged

Conversation

GuillaumeGomez
Copy link
Member

I think it's now time to set this option stable.

r? @rust-lang/docs

@arielb1 arielb1 added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label May 2, 2017
@alexcrichton
Copy link
Member

r? @steveklabnik

@steveklabnik steveklabnik added I-nominated A-docs Area: documentation for any part of the project, including the compiler, standard library, and tools labels May 3, 2017
@steveklabnik
Copy link
Member

Let's talk about this at a doc meeting.

@carols10cents carols10cents added S-waiting-on-team Status: Awaiting decision from the relevant subteam (see the T-<team> label). and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels May 8, 2017
@steveklabnik
Copy link
Member

We decided at the docs meeting that since rustdoc is currently in-progress moving under devtools, we're gonna hold off until then.

@GuillaumeGomez
Copy link
Member Author

😿

@Mark-Simulacrum
Copy link
Member

@steveklabnik Could you elaborate about what "moving under devtools" means? I don't think I've heard of this yet, though I may have missed something...

@GuillaumeGomez
Copy link
Member Author

A new team should be created (soon or not, not sure about this), and in this team, a subteam will officially handle rustdoc changes.

@steveklabnik
Copy link
Member

@Mark-Simulacrum there's some restructuring of some parts of the teams going on, similar to how the new infrastructure team was spun up. Rustdoc's governance is part of this.

@carols10cents carols10cents added T-dev-tools Relevant to the dev-tools subteam, which will review and decide on the PR/issue. and removed A-docs Area: documentation for any part of the project, including the compiler, standard library, and tools labels May 22, 2017
@carols10cents
Copy link
Member

@rust-lang/dev-tools exists now! ping all of yinz!

@Manishearth
Copy link
Member

Manishearth commented May 22, 2017

ono i have 'sponsibilities now

(I'm a tool peer so I don't think I have to sign off on this, but, in case, 👍 from me)

@killercup
Copy link
Member

With great power comes great opportunity to bikeshed! Okay, let me warm up with:

I'd prefer to have CSS written in uppercase in the description. Also, I'm not sure if this actually merits the e shortcut, i.e., if this option is used often enough for that. Usually, -e is for excluding things, not extending.

I'm 👍 on the general idea to stabilize the --extend-css, though.

@nrc nrc added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed I-nominated S-waiting-on-team Status: Awaiting decision from the relevant subteam (see the T-<team> label). labels May 22, 2017
@nrc
Copy link
Member

nrc commented May 22, 2017

We discussed this at the dev-tools meeting today. We were in favour of stabilising. There is a concern that since we do not want to freeze the HTML output by rustdoc, then user css could have different effects from version to version and this might make it more difficult to change rustdoc. However, there is basically no solution to this other than not stabilising the option. So, we think we should accept this PR (r+) on the condition that an extra line of documentation is added to the --help text making clear that there is not stability guarantee about the effect of any given CSS, only that the CSS will be applied.

@GuillaumeGomez
Copy link
Member Author

Ok, will add it as well.

@GuillaumeGomez
Copy link
Member Author

Updated. Is the help message good enough?

@@ -162,8 +162,9 @@ pub fn opts() -> Vec<RustcOptGroup> {
"URL to send code snippets to", "URL")),
stable(optflag("", "markdown-no-toc", "don't include table of contents")),
stable(optopt("e", "extend-css",
"to redefine some css rules with a given file to generate doc with your \
own theme", "PATH")),
"To redefine some css rules with a given file to generate doc with your \
Copy link
Member

@killercup killercup May 23, 2017

Choose a reason for hiding this comment

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

s/redefine/add
s/css/CSS
s/doc/the documentation

I'm not sure about the "add", but I think "redefine" sends the wrong message. You are not changing existing CSS rules, just adding more rules, that may be more specific (because of specificity) and thus appear like they replaced the default rules.

In addition to that it is also totally possible to add new rules for that .fancy-table you used in your docs.

(Sorry to continue do nitpick on this)

Copy link
Member Author

Choose a reason for hiding this comment

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

No problem, it avoids further fixes.

@killercup
Copy link
Member

Thanks. The description looks good!

(Maybe drop the "to" at the beginning but that really doesn't matter 😅)

@GuillaumeGomez
Copy link
Member Author

@bors: r=killercup

@bors
Copy link
Contributor

bors commented May 24, 2017

📌 Commit b4d594f has been approved by killercup

@bors
Copy link
Contributor

bors commented May 24, 2017

⌛ Testing commit b4d594f with merge bb8b661...

@steveklabnik
Copy link
Member

(For posterity, @QuietMisdreavus and I both signed off on this as well)

@bors
Copy link
Contributor

bors commented May 24, 2017

💔 Test failed - status-appveyor

@Mark-Simulacrum
Copy link
Member

@bors retry

  • appveyor timeout

Mark-Simulacrum added a commit to Mark-Simulacrum/rust that referenced this pull request May 24, 2017
…=killercup

Set --extend-css stable

I think it's now time to set this option stable.

r? @rust-lang/docs
bors added a commit that referenced this pull request May 24, 2017
Mark-Simulacrum added a commit to Mark-Simulacrum/rust that referenced this pull request May 25, 2017
…=killercup

Set --extend-css stable

I think it's now time to set this option stable.

r? @rust-lang/docs
@bors
Copy link
Contributor

bors commented May 25, 2017

⌛ Testing commit b4d594f with merge 7f3ed6c...

@Mark-Simulacrum
Copy link
Member

There's very little chance of this succeeding on its own due to appveyor slowness.

@bors retry

@bors
Copy link
Contributor

bors commented May 25, 2017

⌛ Testing commit b4d594f with merge ffb0e2d...

bors added a commit that referenced this pull request May 25, 2017
Set --extend-css stable

I think it's now time to set this option stable.

r? @rust-lang/docs
@bors
Copy link
Contributor

bors commented May 25, 2017

☀️ Test successful - status-appveyor, status-travis
Approved by: killercup
Pushing ffb0e2d to master...

@bors bors merged commit b4d594f into rust-lang:master May 25, 2017
@GuillaumeGomez GuillaumeGomez deleted the extend-css-stable branch May 25, 2017 11:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. T-dev-tools Relevant to the dev-tools subteam, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

10 participants