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

Clean up rustdoc css #40278

Merged
merged 1 commit into from
Mar 11, 2017
Merged

Clean up rustdoc css #40278

merged 1 commit into from
Mar 11, 2017

Conversation

GuillaumeGomez
Copy link
Member

r? @rust-lang/docs

@GuillaumeGomez
Copy link
Member Author

To summarize a bit what I did:

  • Replacement of whitespaces with tabulations.
  • Move color attributes to the "color" file.

@steveklabnik
Copy link
Member

Replacement of whitespaces with tabulations.

I am not a fan of this, but if everyone else wants to, i will not fight it

@GuillaumeGomez
Copy link
Member Author

It's css and it's already the case for almost every line.

@frewsxcv
Copy link
Member

frewsxcv commented Mar 6, 2017

It's css

CSS doesn't need to use tabs. In fact, this is the first time I've seen CSS with tabs.

it's already the case for almost every line

This argument makes more sense though. In general, I'm in favor of spaces over tabs, but in this case, consistency seems more important. We can discuss about potentially migrating to spaces in a different PR.

@ollie27
Copy link
Member

ollie27 commented Mar 7, 2017

src/librustdoc/html/static/rustdoc.css was mostly converted to tabs by #37134. src/doc/rust.css and src/librustdoc/html/static/styles/main.css both use spaces.

@GuillaumeGomez
Copy link
Member Author

CSS doesn't need to use tabs. In fact, this is the first time I've seen CSS with tabs.

Simply because they generally use minifier in order to reduce network impact. However, in CSS it's still the norm to use tabs as far as I can tell.

@bors
Copy link
Contributor

bors commented Mar 9, 2017

☔ The latest upstream changes (presumably #40368) made this pull request unmergeable. Please resolve the merge conflicts.

@GuillaumeGomez
Copy link
Member Author

Updated.

@frewsxcv
Copy link
Member

frewsxcv commented Mar 9, 2017

Have you tested to make sure this doesn't affect the new sidebar changes? If so, r=me

@GuillaumeGomez
Copy link
Member Author

It just moves color setting from one file to another, nothing else. I can still confirm otherwise if you want to be absolutely sure. As your prefer.

@frewsxcv
Copy link
Member

frewsxcv commented Mar 9, 2017

@bors r+ rollup

@bors
Copy link
Contributor

bors commented Mar 9, 2017

📌 Commit 4078b25 has been approved by frewsxcv

arielb1 pushed a commit to arielb1/rust that referenced this pull request Mar 9, 2017
alexcrichton added a commit to alexcrichton/rust that referenced this pull request Mar 10, 2017
alexcrichton pushed a commit to arielb1/rust that referenced this pull request Mar 10, 2017
alexcrichton added a commit to alexcrichton/rust that referenced this pull request Mar 10, 2017
arielb1 pushed a commit to arielb1/rust that referenced this pull request Mar 10, 2017
arielb1 pushed a commit to arielb1/rust that referenced this pull request Mar 10, 2017
arielb1 pushed a commit to arielb1/rust that referenced this pull request Mar 10, 2017
alexcrichton added a commit to alexcrichton/rust that referenced this pull request Mar 11, 2017
@bors bors merged commit 4078b25 into rust-lang:master Mar 11, 2017
@GuillaumeGomez GuillaumeGomez deleted the css-cleanup branch March 11, 2017 15:07
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.

5 participants