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 travis OSX build #4942

Merged
merged 2 commits into from Sep 9, 2017
Merged

Conversation

RX14
Copy link
Contributor

@RX14 RX14 commented Sep 9, 2017

Travis's OSX infastructure is extremely overloaded. We think that the excess of OSX jobs is filling up our travis "build slots" and causing linux builds not to run. To remedy this we've recided to remove OSX from travis, as we already have circleci.

.travis.yml Outdated
matrix:
fast_finish: true
Copy link
Member

Choose a reason for hiding this comment

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

There's no reason to remove fast_finish. We still have multiple builds and notifying the author as soon as one of them fails is still nice to have.

Copy link
Contributor Author

@RX14 RX14 Sep 9, 2017

Choose a reason for hiding this comment

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

I read the fast_finish docs and it made it sound as if it was only valid with allow_failures.

Copy link
Member

@oprypin oprypin Sep 9, 2017

Choose a reason for hiding this comment

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

With fast finishing enabled, Travis CI will mark your build as finished as soon as one of two conditions are met: The only remaining jobs are allowed to fail, or a job has already failed.
https://blog.travis-ci.com/2013-11-27-fast-finishing-builds/

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oops.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed this BTW

@oprypin
Copy link
Member

oprypin commented Sep 9, 2017

I disagree with the big amount of spacing added. Not to mention that it's less clear what the change is.

@RX14
Copy link
Contributor Author

RX14 commented Sep 9, 2017

Why do you dislike the spacing? It's a lot clearer with whitespace between each section.

I also split it into 2 commits so the diff was clear.

@mverzilli mverzilli merged commit 6f651d9 into crystal-lang:master Sep 9, 2017
@mverzilli
Copy link

Let's ping approved PR authors to rebase so they don't block the queues

@oprypin
Copy link
Member

oprypin commented Sep 9, 2017

I don't think that's needed. Config is not taken from PRs, that's insecure

@RX14
Copy link
Contributor Author

RX14 commented Sep 9, 2017

@oprypin so why did this PR not build OSX?

@oprypin
Copy link
Member

oprypin commented Sep 9, 2017

Hm nevermind. But I still think everything will work out without manual rebasing. PRs get rebased anyway.

@mverzilli mverzilli added this to the Next milestone Sep 9, 2017
@RX14
Copy link
Contributor Author

RX14 commented Sep 9, 2017

@oprypin but we want green CI before they're rebased onto master.

@oprypin
Copy link
Member

oprypin commented Sep 9, 2017

Ah, so this is about PRs that failed the last time they were checked. Have you tried simply restarting the build in the Travis interface?

@RX14
Copy link
Contributor Author

RX14 commented Sep 9, 2017

@oprypin no, this is about fixing PRs hanging in the "queueing" status for several days.

@mverzilli
Copy link

Restarting the build in Travis runs on the targets defined before this PR. But I think that's ok really. Let's see in a case by case basis, as needed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants