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.

Sign up for GitHub

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

Jump to bottom

Remove trailing whitespace at the end of lines #32235

Merged
merged 1 commit into from
Mar 28, 2016
Merged

Remove trailing whitespace at the end of lines #32235

merged 1 commit into from
Mar 28, 2016

Conversation

floftar
Copy link
Contributor

@floftar floftar commented Mar 13, 2016

According the rules of styling Rust code there should not be trailing whitespace at the end of lines or files. I thought that it might be good to remove trailing whitespace from other files also, if it does not break anything.

hanna-kruppe
hanna-kruppe reviewed Mar 13, 2016
View reviewed changes
src/libbacktrace/Makefile.am
@@ -6,12 +6,12 @@
# met:

# (1) Redistributions of source code must retain the above copyright
# notice, this list of conditions and the following disclaimer.
Copy link
Contributor

Choose a reason for hiding this comment

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

AFAIK libbracktrace is not part of Rust proper, so the Rust styleguide doesn't apply (and these files shouldn't be modified anyway if at all possible, because it complicates upgrades).

Copy link
Member

Choose a reason for hiding this comment

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

Be careful with markdown files! I can't give this a proper review right now, but trailing white space is significant in them, it's used in the book intentionally at least once.

On Mar 13, 2016, 16:16 -0400, rkruppenotifications@github.com, wrote:

Insrc/libbacktrace/Makefile.am(#32235 (comment)):

@@ -6,12 +6,12 @@># met:>># (1) Redistributions of source code must retain the above copyright>-# notice, this list of conditions and the following disclaimer.

libbracktrace is not part of Rust proper, so the Rust styleguide doesn't apply (and these files shouldn't be modified anyway if at all possible, because it complicates upgrades).

—
Reply to this email directly orview it on GitHub(https://github.com/rust-lang/rust/pull/32235/files#r55942955).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thank you for your comments! This change can be abandoned and be split for easier reviewing. Sorry about making a big change. Of course if trailing whitespaces are not a problem at all, then change does not have to be made in first place.

Copy link
Member

Choose a reason for hiding this comment

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

I think that generally, removing it is good, but it can't be fully done with a script, eg, for reasons like both of these.

I agree with @rkruppe about everything in libbacktrace. I'll review the rest now.

@steveklabnik
Copy link
Member

Okay, yeah, all of this looks fine, except libbacktrace.

@floftar
Copy link
Contributor Author

floftar commented Mar 14, 2016

There should not be anymore changes to libbacktrace.

@steveklabnik
Copy link
Member

@bors: r+ rollup

thanks so much!

@bors
Copy link
Contributor

bors commented Mar 14, 2016

📌 Commit dcadf29 has been approved by steveklabnik

@floftar
Copy link
Contributor Author

floftar commented Mar 14, 2016

Thanks for comments. I will try to make smaller commits in the future to make reviews easier.

@bors
Copy link
Contributor

bors commented Mar 15, 2016

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

@bors
Copy link
Contributor

bors commented Mar 22, 2016

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

@floftar
Copy link
Contributor Author

floftar commented Mar 23, 2016

I rebased this pull request.

@sanxiyn
Copy link
Member

sanxiyn commented Mar 28, 2016

@bors r+ rollup

@bors
Copy link
Contributor

bors commented Mar 28, 2016

📌 Commit ccafdae has been approved by sanxiyn

steveklabnik added a commit to steveklabnik/rust that referenced this pull request Mar 28, 2016
@steveklabnik
Rollup merge of rust-lang#32235 - fbergr:trailing_whitespace, r=sanxiyn
56bb7b6 
Remove trailing whitespace at the end of lines

According the rules of styling Rust code there should not be trailing whitespace at the end of lines or files. I thought that it might be good to remove trailing whitespace from other files also, if it does not break anything.
@steveklabnik steveklabnik mentioned this pull request Mar 28, 2016
Merged
bors added a commit that referenced this pull request Mar 28, 2016
@bors
Auto merge of #32561 - steveklabnik:rollup, r=steveklabnik
221c940 
Rollup of 7 pull requests

- Successful merges: #32177, #32235, #32472, #32504, #32507, #32509, #32534
- Failed merges:
@bors bors merged commit ccafdae into rust-lang:master Mar 28, 2016
@floftar floftar deleted the trailing_whitespace branch March 29, 2016 03:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@steveklabnik steveklabnik

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@floftar @steveklabnik @bors @sanxiyn @hanna-kruppe