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

Remove remaining trailing whitespace #9792

Merged
merged 4 commits into from
Jan 19, 2015
Merged

Remove remaining trailing whitespace #9792

merged 4 commits into from
Jan 19, 2015

Conversation

jakebolewski
Copy link
Member

Removes TODOs from #9787.

@tkelman
Copy link
Contributor

tkelman commented Jan 18, 2015

I rebased this for you, hope you don't mind.

@jakebolewski
Copy link
Member Author

Not at all. Here is the command I used if you need to do this on the 0.3 branch.

find . -type f \(-name “*.inc” -o -name “*.make” -o -name “*.md” -o -name “*.rst” -o -name “*.sh” -o -name "*.yml" -o  -name "*Makefile" \) -exec perl -p -i -e "s/[ \t]*$//g" "{}" \;

@tkelman
Copy link
Contributor

tkelman commented Jan 18, 2015

That is useful, thanks. Haven't done that yet but have been thinking about it to simplify backporting.

@tkelman
Copy link
Contributor

tkelman commented Jan 18, 2015

Nevermind, on non-OSX it apparently needs a space after the open parenthesis, and non-unicode quotes are probably also a good idea.

@jakebolewski
Copy link
Member Author

Weird, seems to be an autocorrect "feature" in Apple's Notes App.

tkelman added a commit that referenced this pull request Jan 19, 2015
ref PR #9792 for command to do this
tkelman added a commit that referenced this pull request Jan 19, 2015
ref PR #9792 for command to do this
tkelman added a commit that referenced this pull request Jan 19, 2015
ref PR #9792 for command to do this
catch a few recent whitespace additions
@tkelman
Copy link
Contributor

tkelman commented Jan 19, 2015

Rebased this again. A few new whitespaces have snuck in. Interestingly the check does not cause Travis to fail immediately, it runs all the tests and forces you to scroll all the way up to the start of the log to figure out why the status was failed. I'm liking the option of doing this in a separate Travis job or on a separate Travis-competitor service (#9553 did the former, and mentions the latter) more and more.

@ViralBShah
Copy link
Member

I cancelled the appveyor build. Assume that is ok here.

@tkelman
Copy link
Contributor

tkelman commented Jan 19, 2015

Yeah, if there are better things to run it on sure. It was otherwise idle, and just in case...

@ViralBShah
Copy link
Member

I wanted my sparse solver stuff to go through...

@tkelman
Copy link
Contributor

tkelman commented Jan 19, 2015

Sure, looks like it needs some work to deal with the recent Triangular changes.

Looking at this more closely, we could potentially do make check-whitespace || exit 1 in .travis.yml to make it stop early on whitespace problems. That's what the check as written currently does, where it's entirely in bash straight from .travis.yml - when it gets moved into a script that gets run by make, then the exit inside the script just sets the exit code of that line, rather than terminating the Travis job early.

@jakebolewski
Copy link
Member Author

Can we move the check to the end of the travis run, or add your suggested fix, and be done? I don't think we have to be too fancy yet. I think that the fast fail is nice, but I agree now it penalizes our appveyor ci builds, so putting it at the end makes sense.

@tkelman
Copy link
Contributor

tkelman commented Jan 19, 2015

Sure. My idea works for fast failure, tested it in a different branch: https://travis-ci.org/JuliaLang/julia/builds/47524519

Just have to pick which we want. If we merge #9809 as well then doing it at the end of the tests makes more sense, since it should happen earlier on people's local machines.

jakebolewski added a commit that referenced this pull request Jan 19, 2015
Remove remaining trailing whitespace
@jakebolewski jakebolewski merged commit 3046490 into master Jan 19, 2015
@jakebolewski jakebolewski deleted the jcb/rmremainingws branch January 19, 2015 17:24
@jakebolewski
Copy link
Member Author

@tkelman thanks, I just kept the behavior we currently have. We can always tweak this latter.

vchuravy pushed a commit to JuliaLang/LazyArtifacts.jl that referenced this pull request Oct 2, 2023
ref PR JuliaLang/julia#9792 for command to do this
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.

3 participants