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

ci: Run rustfmt on x86_64-linux only #1814

Merged
merged 1 commit into from
Apr 29, 2019

Conversation

tesuji
Copy link
Contributor

@tesuji tesuji commented Apr 27, 2019

Also removing obsolete PATH="$HOME/rust/bin:$PATH".

Copy link
Contributor

@kinnison kinnison left a comment

Choose a reason for hiding this comment

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

Does this honestly save a lot of CI time? I'd rather we ran cargo fmt everywhere just in case it catches something that conditional compilation made hard for it to spot unless it's consuming a huge quantity of time.

It takes less than a second to check on my slowest computer for example, so even if it took 5s on each of our platforms, that's hardly a lot of time.

What is your justification for this change?

appveyor.yml Show resolved Hide resolved
@tesuji
Copy link
Contributor Author

tesuji commented Apr 28, 2019

I would say this doesn't save noticeable build time. The rustfmt
on Linux builds are same program (same executable, same architecture).
So I personally don't want to run it multiple times. I agree with you on
keeping rustfmt running on Windows.

@kinnison
Copy link
Contributor

Okay, so perhaps one rustfmt on linux, one on macos, and one on windows?

@tesuji tesuji force-pushed the rustfmt-x86_64 branch 3 times, most recently from e6fb049 to 07a6b64 Compare April 29, 2019 18:38
@kinnison kinnison merged commit 1cf31c1 into rust-lang:master Apr 29, 2019
@tesuji tesuji deleted the rustfmt-x86_64 branch April 29, 2019 19:47
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.

2 participants