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

Failures in --pre builds #3228

Closed
hmaarrfk opened this issue Jun 27, 2018 · 11 comments
Closed

Failures in --pre builds #3228

hmaarrfk opened this issue Jun 27, 2018 · 11 comments

Comments

@hmaarrfk
Copy link
Member

This first showed up in the windows builds failing.
this was due to to the fact that the --pre flag was enabled by default.

PR #3222 attempts to make this more obvious by dedicating a --pre build to the build matrix.

I have a hunch it is due to numpy 1.15.rc1

On travis this wasn't showing up for some reason. I triggered a build on my fork of master (updated) and the dedicated --pre build fails to pull in the numpy 1.15.rc1 dependency from upstream https://travis-ci.org/hmaarrfk/scikit-image/jobs/397451046#L861

When this PR builds, #3227, travis will pull in prereleases from pypi too the problem should show up for travis too.
I've found that while the --pre build compiles with numpy 1.15.rc1, it fails some tests on linux.

@stefanv
Copy link
Member

stefanv commented Jun 27, 2018

Travis-CI does fail against the latest pre-release! See #3204

@hmaarrfk
Copy link
Member Author

Good catch. Haven't had a chance to keep up with the PRs lately.
I think the common issue that #3204 and #3227 address is that the following line is executed without the $PIP_FLAGS
https://github.com/scikit-image/scikit-image/blob/master/tools/travis/before_install.sh#L63

@hmaarrfk
Copy link
Member Author

@stefanv, how would you feel about allowing failures in the --pre builds?

I have a few branches that would show how it looks like on my fork:

The idea would be that contributors can see that their builds are actually passing on supported machines, while the more advanced devs and maintainers can check to see if scikit-image is working on builds such as --pre without having everything look red all the time.

I don't like the fact that the color is "green", I would have preferred a colour such as "blue", but then again, maybe that isn't ideal either.

@jni
Copy link
Member

jni commented Jun 28, 2018

@hmaarrfk whoa, very cool!

I'm not sure how I feel about it, but it's a tentative 👍 from me. It makes a lot of sense to not allow other projects to change our status to red. It just means one more tick box for reviewers in the PR checklist.

@stefanv
Copy link
Member

stefanv commented Jun 29, 2018

If we allow these failures, I am pretty sure no one will ever look at them.

Perhaps we should only do the --pre installs on our nightly builds, and have those emails go out to a wider audience?

@hmaarrfk
Copy link
Member Author

The nightly builds seem like an appropriate place for them.

@jni
Copy link
Member

jni commented Jul 1, 2018

@stefanv I have more faith in our reviewers than you do. ;) If we have a tick mark, and if we make some reviewer guidelines to always check, I think it's fine to trust people to follow process, just like we trust them to read the diff before merging.

@jni
Copy link
Member

jni commented Jul 1, 2018

(Incidentally, the process would be to check allowed failures and the reviewer should raise an issue when a failure is observed)

There is probably also a reasonable way to check those failures automatically and notify "the authorities". =P

@hmaarrfk
Copy link
Member Author

hmaarrfk commented Jul 1, 2018

I mean, this --pre thing will come to bite us eventually, we know about it, and will work toward fixing it.

I think allowing temporary failures in the --pre builds while we undergo those fixes is a good way to go about it.
The PR that makes the --pre build pass, should simply cause future PRs to failre on --pre

Disabling and enabling the "allowed failures" is only one line of travis/appveyor script.

@sciunto sciunto added this to the 0.15 milestone Feb 1, 2019
@soupault soupault modified the milestones: 0.15, 0.16 Apr 20, 2019
@jni jni modified the milestones: 0.16, 0.17 Sep 17, 2019
@jni
Copy link
Member

jni commented Sep 17, 2019

The issue of what to do about --pre failures remains open, but it shouldn't block 0.16, so I've changed the milestone on this.

@grlee77
Copy link
Contributor

grlee77 commented Apr 5, 2021

I am closing this as #3222 was merged and the discussion related to Travis is now outdated. If desired, please reopen a new issue about potentially allowing --pre failures, which I don't think we have done at this time

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

No branches or pull requests

6 participants